-
Notifications
You must be signed in to change notification settings - Fork 452
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: (Blueprint) Allow customization of setup sa key #1065
feat: (Blueprint) Allow customization of setup sa key #1065
Conversation
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.
Thanks for the PR @diegolnasc
@@ -40,6 +40,7 @@ const setupKeyOutputName = "sa_key" | |||
type TFBlueprintTest struct { | |||
discovery.BlueprintTestConfig // additional blueprint test configs | |||
name string // descriptive name for the test | |||
saKey string // optional setup encoded sa key |
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.
If we are specifying an explicit SA key, I would expect it to be already decoded (i.e not b64 encoded) since thats how you would download/generate it via gcloud. WDYT?
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 agree.
My script was already generating base64 so I ended up skewing it.
@@ -205,6 +210,19 @@ func (b *TFBlueprintTest) getTFOutputsAsInputs(o map[string]interface{}) map[str | |||
return n | |||
} | |||
|
|||
// loadSetupSAKey activate a gcp encoded sa key | |||
func (b *TFBlueprintTest) loadSetupSAKey(saKey string, isEncoded bool) { |
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.
Instead of having the isEncoded logic here, I'd just keep it on line 164.
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 thought it would be cleaner in func loadSetupSAKey.
Well, I'm going to delete the func because it will lose the sense of existing just to call gcloud cmd.
Make setup sa key more flexible for other tests.