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
Design doc for private bicep registries #53
Design doc for private bicep registries #53
Changes from 6 commits
305b117
34b512a
81674fb
b550104
7dcab38
d2856f6
dfd226f
fc93b70
e1efb0b
f577158
70d68a0
7755331
3565b5e
10f3abd
c8b49d2
a5f3f29
fa3c09a
34bd19d
a29a128
6099c37
5b8398e
b4e3825
a973750
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
Which component will be responsible for these steps?
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.
as discussed in the meeting, Driver(ReadFromRegistry) will be responsible for getting the AAD and acr refresh tokens and creating ORAS auth client.
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.
Can you please update the doc to make it explicit and the architecture diagram to reflect this call flow?
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.
We'd usually call this
kind
to avoid overloading the termtype
. Every resource has atype
that returns it's resource type, so we try to avoid using that field name for other purposes.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.
Will this be Bicep-specific? And is
type
going to be a required field?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.
Have we considered either making it more generic - use
federatedIdentity
for both aws and azure, or make it more specificazureWorkloadIdentity
because that's what we are supporting. @willtsai @Reshrahim thoughts?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.
I think making it more specific to the cloud providers like
azureWorkloadIdentity
andawsIRSA
would be easier for the users to interpret. Also expanding IRSA to IAM Roles for Service accounts in product/code docs would be helpful.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.
So this means that the user continues to set up their registry auth secrets as
generic
type like they do today? If so, I'm good with this approach.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.
I don't think that either option is bad, but I'm curious to hear what was discussed last week.
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.
should these actually be classified as secrets or can they be a little less strict to allow for better troubleshooting for the user?