Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NE-1323: Add AWS RoleARN for Shared VPC support #195
NE-1323: Add AWS RoleARN for Shared VPC support #195
Changes from all commits
d5ce581
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW I am planning to add a section on using a Shared VPC to OpenShift's docs: https://docs.openshift.com/container-platform/4.13/networking/external_dns_operator/nw-creating-dns-records-on-aws.html
If you think we don't need both (this repo docs and OpenShift docs), I can drop this one. But I figure better safe than sorry.
I know the docs in this repo are not just for OpenShift users, but it does feel a little strange there isn't a mention of the Cloud Credential Operator providing the credentials for OpenShift anywhere. I wonder if we should introduce that concept somewhere around here, as a new user, that wasn't clear to me I didn't need credentials from reading these docs.
I feel like we should also have a
aws-openshift.md
doc just like the azure and gcp openshift docs in https://github.com/openshift/external-dns-operator/blob/main/docs. That's seems odd to me.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OCP docs are always the priority over this repo.
I believe that the initial purpose of this doc was to show how to configure the DNS providers: format of the secrets, configuration and tuning options. Basically anything that lays in
spec.provider
which differs from provider to provider, not only credentials. It all started from more exotic providers like BlueCat and Inflobox which could not be used in CCO context. That's whyCredentials for DNS providers
chapter talks about ExternalDNS Operator not provisioning the credentials.I think I agree that we need to mention
CredentialsRequest
for the big 3 providers and keep the mention about Infoblox and BlueCat as not managed by CCO. That we can do in this PR I guess or feel free to submit another PR with a dedicated.md
for aws.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay thanks. I'll leave what I have now, and possibly open another PR later with details on
CredentialsRequest
to keep this PR cleaner.