Skip to content
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.

Sign up for GitHub

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

Jump to bottom

parse resource name before removing deleted secret #17004

Merged
merged 2 commits into from
Oct 25, 2017
Merged

parse resource name before removing deleted secret #17004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a17af0e
parse resource name before removing deleted secret
juanvallejo Oct 23, 2017
b00c5d1
validate user-specified resource, if given
juanvallejo Oct 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 23 additions & 1 deletion pkg/oc/cli/secrets/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io"
"io/ioutil"
"os"
"strings"

kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -68,6 +69,16 @@ func (o SecretOptions) Validate() error {
return errors.New("KubeCoreClient must be present")
}

// if any secret names are of the form <resource>/<name>,
// ensure <resource> is a secret.
for _, secretName := range o.SecretNames {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't this using standard resource builder?

Copy link
Contributor Author

@juanvallejo juanvallejo Oct 30, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't this using standard resource builder?

The resource builder is used in the GetSecrets method. The code block here validates that a resource kind is secret if it is specified as <kind>/<name>, before getting to the builder, which ignores NotFound errors in order to allow deleted secrets to still be unlinked. Without this check, if a user provides a command such as: oc secret unlink sa_name pods/not_a_secret, this line would create a secret with the name pods/not_a_secret.

if segs := strings.Split(secretName, "/"); len(segs) > 1 {
if segs[0] != "secret" && segs[0] != "secrets" {
return errors.New(fmt.Sprintf("expected resource of type secret, got %q", secretName))
}
}
}

return nil
}

Expand Down Expand Up @@ -98,11 +109,22 @@ func (o SecretOptions) GetServiceAccount() (*kapi.ServiceAccount, error) {
func (o SecretOptions) GetSecretNames(secrets []*kapi.Secret) sets.String {
names := sets.String{}
for _, secret := range secrets {
names.Insert(secret.Name)
names.Insert(parseSecretName(secret.Name))
}
return names
}

// parseSecretName receives a resource name as either
// <resource type> / <name> or <name> and returns only the resource <name>.
func parseSecretName(name string) string {
segs := strings.Split(name, "/")
if len(segs) < 2 {
return name
}

return segs[1]
}

// GetMountSecretNames Get a list of the names of the mount secrets associated
// with a service account
func (o SecretOptions) GetMountSecretNames(serviceaccount *kapi.ServiceAccount) sets.String {
Expand Down
13 changes: 13 additions & 0 deletions test/cmd/secrets.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,19 @@ os::cmd::expect_success 'oc secrets add deployer basicauth sshauth --for=pull'
# make sure we can add as as pull secret and mount secret at once
os::cmd::expect_success 'oc secrets add deployer basicauth sshauth --for=pull,mount'

# attach secrets to service account
# test that those secrets can be unlinked
# after they have been deleted.
os::cmd::expect_success 'oc create secret generic deleted-secret'
os::cmd::expect_success 'oc secrets link deployer deleted-secret'
# confirm our soon-to-be-deleted secret has been linked
os::cmd::expect_success_and_text "oc get serviceaccount deployer -o jsonpath='{.secrets[?(@.name==\"deleted-secret\")]}'" 'deleted\-secret'
# delete "deleted-secret" and attempt to unlink from service account
os::cmd::expect_success 'oc delete secret deleted-secret'
os::cmd::expect_failure_and_text 'oc secrets unlink deployer secrets/deleted-secret' 'Unlinked deleted secrets'
# ensure already-deleted secret has been unlinked
os::cmd::expect_success_and_not_text "oc get serviceaccount deployer -o jsonpath='{.secrets[?(@.name==\"deleted-secret\")]}'" 'deleted\-secret'

# attach secrets to service account
# single secret with prefix
os::cmd::expect_success 'oc secrets link deployer basicauth'
Expand Down