Add secret-ref flag to create source git #350
Conversation
| secretName := name | ||
| if sourceGitSecretRef != "" { | ||
| secretName = sourceGitSecretRef | ||
| } |
There was a problem hiding this comment.
Instead of overwriting the generated secret here, I would expect the command to not generate and upsert a secret at all if an existing secret name is given.
There was a problem hiding this comment.
So drop scenario 2 above? Can do if that's what you prefer!
There was a problem hiding this comment.
It does not do scenario 2 at the moment, as on L234 the secret that is upserted does not take the configured value into account. If you make it do this, I am fine with the change.
There was a problem hiding this comment.
🤦 Oops missed it, sorry. Gimme a mo
There was a problem hiding this comment.
Adding just the name in there would not be sufficient by the way, as this would result in the overwrite of existing data.
It would require some additional logic to only create (and not update) a secret with the given name if it does not exist yet.
There was a problem hiding this comment.
What's the current behaviour if a secret with the same name as the git source exists? Would that not have the same behavior?
Maybe I should just simplify this to just scenario 1?
There was a problem hiding this comment.
Let me know what you would prefer :)
There was a problem hiding this comment.
Please drop option 2. I see no reason why would you want to setup a name for the generated secret. When secret-ref is specified, then we assume it's up to the user to create it on the cluster by other means than gotk. If we make this change then it should be for all create source commands not only Git.
cmd/gotk/create_source_git.go
Outdated
| @@ -105,6 +106,7 @@ func init() { | |||
| createSourceGitCmd.Flags().Var(&sourceGitKeyAlgorithm, "ssh-key-algorithm", sourceGitKeyAlgorithm.Description()) | |||
| createSourceGitCmd.Flags().Var(&sourceGitRSABits, "ssh-rsa-bits", sourceGitRSABits.Description()) | |||
| createSourceGitCmd.Flags().Var(&sourceGitECDSACurve, "ssh-ecdsa-curve", sourceGitECDSACurve.Description()) | |||
| createSourceGitCmd.Flags().StringVarP(&sourceGitSecretRef, "secret-ref", "s", "", "the name to use for the source secret, or an existing secret containing SSH or basic credentials") | |||
There was a problem hiding this comment.
Please remove the s shorthand, this conflict with --silent.
|
Hope you all had a good weekend! I've removed changing the name of generated secrets and added the same flag to Helm and Bucket sources. Let me know if you need any other changes |
|
@allymparker you need to rerun the docs gen |
|
I think that's sorted @stefanprodan |
|
@allymparker any idea why cbba648 seems to touch all LOC? Is it a disguised merge commit? |
There was a problem hiding this comment.
LGTM
Thanks @allymparker can you please squash all your commits into one and rebase please?
stefanprodan
added
area/source
Add secret-ref flag to Helm source Add secret-ref to bucket source
|
@stefanprodan done, thanks! @hiddeco sorry, I wasn't sure, might have been the rebases, but it seems to be happier after squashing |
What?
Add secret-ref flag to
create sourceoptionsWhy?
This allows passing the name of an existing secret that already exists in the cluster (perhaps created with terraform)
Tested?
make testis passingManually tested with
--export