Add support for setting environments secrets #3769
Conversation
| @@ -75,7 +75,7 @@ and talk through which code gets run in order. | |||
| This task might be tricky. Typically, gh commands do things like look up information from the git repository | |||
| in the current directory, query the GitHub API, scan the user's `~/.ssh/config` file, clone or fetch git | |||
| repositories, etc. Naturally, none of these things should ever happen for real when running tests, unless | |||
| you are sure that any filesystem operations are stricly scoped to a location made for and maintained by the | |||
| you are sure that any filesystem operations are strictly scoped to a location made for and maintained by the | |||
There was a problem hiding this comment.
Unrelated, a small typo I noticed.
| @@ -115,7 +123,8 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command | |||
| return setRun(opts) | |||
| }, | |||
| } | |||
| cmd.Flags().StringVarP(&opts.OrgName, "org", "o", "", "List secrets for an organization") | |||
There was a problem hiding this comment.
I believe this description was wrong, fixed it while I was touching the code around.
pkg/cmd/secret/set/set.go
Outdated
| } else { | ||
| err = putRepoSecret(client, pk, baseRepo, opts.SecretName, encoded) | ||
| if envName != "" { |
There was a problem hiding this comment.
Not sure if it's the most idiomatic way of writing this, let me know if you prefer it as:
if orgName != "" {
err = putOrgSecret(client, host, pk, *opts, encoded)
} else if envName != "" {
err = putEnvSecret(client, pk, baseRepo, envName, opts.SecretName, encoded)
} else {
err = putRepoSecret(client, pk, baseRepo, opts.SecretName, encoded)
}
There was a problem hiding this comment.
Collapsing that nested conditional nto the else as you suggested would be a welcome change; it's so minor I'll just push a commit before I merge.
| @@ -204,6 +216,46 @@ func Test_setRun_repo(t *testing.T) { | |||
| assert.Equal(t, payload.EncryptedValue, "UKYUCbHd0DJemxa3AOcZ6XcsBwALG9d4bpB8ZT0gSV39vl3BHiGSgj8zJapDxgB2BwqNqRhpjC4=") | |||
| } | |||
|
|
|||
| func Test_setRun_env(t *testing.T) { | |||
There was a problem hiding this comment.
This is almost like Test_setRun_repo above, except the 2nd mock and the EnvName option... Might be better to parametrize the existing test?
There was a problem hiding this comment.
I think this is fine for now!
| @@ -204,6 +216,46 @@ func Test_setRun_repo(t *testing.T) { | |||
| assert.Equal(t, payload.EncryptedValue, "UKYUCbHd0DJemxa3AOcZ6XcsBwALG9d4bpB8ZT0gSV39vl3BHiGSgj8zJapDxgB2BwqNqRhpjC4=") | |||
| } | |||
|
|
|||
| func Test_setRun_env(t *testing.T) { | |||
There was a problem hiding this comment.
I think this is fine for now!
pkg/cmd/secret/set/set.go
Outdated
| } else { | ||
| err = putRepoSecret(client, pk, baseRepo, opts.SecretName, encoded) | ||
| if envName != "" { |
There was a problem hiding this comment.
Collapsing that nested conditional nto the else as you suggested would be a welcome change; it's so minor I'll just push a commit before I merge.
Support to list environemnt secrets was added in #3270 but it would be nice to also be able to set them.
Fixes #3265