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
add BackendSecurityPolicy for traffic (authN/authZ) from gateway to provider #43
add BackendSecurityPolicy for traffic (authN/authZ) from gateway to provider #43
Changes from 36 commits
08be1f7
2ac7698
5ff638f
c625ebe
e200702
43f77fc
ea69193
7e9315c
57e0fa1
bc0dadd
b071160
d2f72ed
52ef4f9
7b811b7
7dbb202
c04ec5e
e977a3d
a692194
41db68a
6ea6f94
3e1d587
2a42311
191f9a7
fbf1c27
b0cd4ca
3d9ff5a
c91b45f
c5237b3
ddfcb33
130b1bf
6c68980
ef3158c
5ff45c2
1f5e736
658c1dd
2bcda7a
f7ee160
c4f7baf
6b8f698
1d71e7a
70a4b29
99e8ae3
70aa411
27df777
dbc37e2
21db108
480b26b
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.
well, shouldn't the type be "CloudProviders" instead of AWS_IAM? The "type" field is for union type assertion and this feels a weird because AWS_IAM is not for the union type assertion but for the higher layer. @yuzisun @arkodg
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.
This is relevant to the thread to #43 (comment) I am still not convinced by the "Yaml UX" stuff there. I am fine with that but at least to be consistent could you make this CloudProviders and have a type inside it for a specific cloud provider? (which i think is a worse UX vs the flat one)
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've flattened it out per req
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 i wonder what kind of logic will make cloud providers special compared to others (currently only apiKey though). how do you envision this additional indirect layer will be used? in other words, what kind of logic will be shared among cloud providers
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.
see https://docs.aws.amazon.com/IAM/latest/UserGuide/id_credentials_temp.html#:~:text=You%20can%20use%20the%20AWS,has%20permissions%20to%20do%20so.
https://cloud.google.com/docs/authentication#credentials
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.
yeah i get that it's complex, but my question is why this another layer is needed and what kind of logic will be shared among cloud providers? what becomes difficult to achieve when you have
*AWSCredentials
directly here without another layer ofAuthenticationCloudProviderCredentials
. How does havingAuthenticationCloudProviderCredentials
helps implement various cloud providers vs having them in hereThere 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.
This is mainly a yaml UX, the additional layer for grouping is there for readability, if we list the individual types then it is not intuitive for user to see the classification. The cloud provider security implementations are different but they share the same high level concepts.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.