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
feat: Adds GCP secret manager triggerauthentication #4864
feat: Adds GCP secret manager triggerauthentication #4864
Changes from 2 commits
3cfe0ea
3c859d7
e92ea50
2571061
e615642
46f2ded
37b2614
c40ebe1
5657319
fab71a5
741c43f
d934f15
ef01e8a
02ada6d
35a4566
078cddf
2cc5ff7
df99446
e9ca060
b602044
c9ea398
0951b60
57a3f98
295cecf
3e8f5e4
7d7b9c6
2bf8794
7a2448b
0e0bb5e
9211d40
1a2526b
6b6ce41
c779831
99e11f4
0c4873b
a9d7c72
86f9cf5
578a342
1da328f
d70edbe
6c98fb0
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.
Maybe it's a stupid question, but, can't we infer this value from credentials or pod identity? AFAIR, credentials json contains the project, and we can query the project to metadata api or reading the (not GCP) workload webhook
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.
You're right, for a credentials JSON file it is certainly possible to parse the project ID from it, however, for pod identity I am not sure if we can get the project ID (or how to get it). That was the intuition behind making the field mandatory (for both credentials JSON input and pod identity based authentication), to avoid confusion.
If you can share an approach for getting the project ID when using pod identity, I will remove this 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.
Using PodIdentity you can get it too using metadata client or another already set env (metadata is available if your ar inside GCP and the env is available if you are outside GCP because the mutating webhook adds it).
You have an example here: https://github.com/kedacore/keda/blob/main/pkg/scalers/gcp_stackdriver_client.go#L84
If you have any doubt just ask 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.
Hey @JorTurFer, I have made the requisite changes PTAL, TY!
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.