From 7e079f5144474cae05802771f604df2b748d781f Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Tue, 19 Oct 2021 16:12:31 -0700 Subject: [PATCH] --as-uid flag in kubectl and kubeconfigs. This corresponds to previous work to allow impersonating UIDs: * Introduce Impersonate-UID header: #99961 * Add UID to client-go impersonation config #104483 Signed-off-by: Margo Crawford --- .../pkg/genericclioptions/config_flags.go | 9 +++ .../client-go/tools/clientcmd/api/types.go | 5 +- .../client-go/tools/clientcmd/api/v1/types.go | 7 +- .../api/v1/zz_generated.conversion.go | 2 + .../tools/clientcmd/client_config.go | 2 + .../tools/clientcmd/client_config_test.go | 43 +++++++++++ .../client-go/tools/clientcmd/overrides.go | 4 ++ .../client-go/tools/clientcmd/validation.go | 6 +- .../tools/clientcmd/validation_test.go | 72 +++++++++++++++++++ test/cmd/authorization.sh | 10 +++ test/cmd/kubeconfig.sh | 47 ++++++++++++ 11 files changed, 201 insertions(+), 6 deletions(-) diff --git a/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go b/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go index 8492e8601bf..7508e47c4ec 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go +++ b/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go @@ -48,6 +48,7 @@ const ( flagCAFile = "certificate-authority" flagBearerToken = "token" flagImpersonate = "as" + flagImpersonateUID = "as-uid" flagImpersonateGroup = "as-group" flagUsername = "username" flagPassword = "password" @@ -94,6 +95,7 @@ type ConfigFlags struct { CAFile *string BearerToken *string Impersonate *string + ImpersonateUID *string ImpersonateGroup *[]string Username *string Password *string @@ -171,6 +173,9 @@ func (f *ConfigFlags) toRawKubeConfigLoader() clientcmd.ClientConfig { if f.Impersonate != nil { overrides.AuthInfo.Impersonate = *f.Impersonate } + if f.ImpersonateUID != nil { + overrides.AuthInfo.ImpersonateUID = *f.ImpersonateUID + } if f.ImpersonateGroup != nil { overrides.AuthInfo.ImpersonateGroups = *f.ImpersonateGroup } @@ -336,6 +341,9 @@ func (f *ConfigFlags) AddFlags(flags *pflag.FlagSet) { if f.Impersonate != nil { flags.StringVar(f.Impersonate, flagImpersonate, *f.Impersonate, "Username to impersonate for the operation. User could be a regular user or a service account in a namespace.") } + if f.ImpersonateUID != nil { + flags.StringVar(f.ImpersonateUID, flagImpersonateUID, *f.ImpersonateUID, "UID to impersonate for the operation.") + } if f.ImpersonateGroup != nil { flags.StringArrayVar(f.ImpersonateGroup, flagImpersonateGroup, *f.ImpersonateGroup, "Group to impersonate for the operation, this flag can be repeated to specify multiple groups.") } @@ -410,6 +418,7 @@ func NewConfigFlags(usePersistentConfig bool) *ConfigFlags { CAFile: stringptr(""), BearerToken: stringptr(""), Impersonate: stringptr(""), + ImpersonateUID: stringptr(""), ImpersonateGroup: &impersonateGroup, usePersistentConfig: usePersistentConfig, diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go index 31716abf180..44a2fb94b15 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go @@ -124,7 +124,10 @@ type AuthInfo struct { // Impersonate is the username to act-as. // +optional Impersonate string `json:"act-as,omitempty"` - // ImpersonateGroups is the groups to imperonate. + // ImpersonateUID is the uid to impersonate. + // +optional + ImpersonateUID string `json:"act-as-uid,omitempty"` + // ImpersonateGroups is the groups to impersonate. // +optional ImpersonateGroups []string `json:"act-as-groups,omitempty"` // ImpersonateUserExtra contains additional information for impersonated user. diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/types.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/types.go index 7e8351032cf..757ed817b29 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/types.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/types.go @@ -111,10 +111,13 @@ type AuthInfo struct { // TokenFile is a pointer to a file that contains a bearer token (as described above). If both Token and TokenFile are present, Token takes precedence. // +optional TokenFile string `json:"tokenFile,omitempty"` - // Impersonate is the username to imperonate. The name matches the flag. + // Impersonate is the username to impersonate. The name matches the flag. // +optional Impersonate string `json:"as,omitempty"` - // ImpersonateGroups is the groups to imperonate. + // ImpersonateUID is the uid to impersonate. + // +optional + ImpersonateUID string `json:"as-uid,omitempty"` + // ImpersonateGroups is the groups to impersonate. // +optional ImpersonateGroups []string `json:"as-groups,omitempty"` // ImpersonateUserExtra contains additional information for impersonated user. diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go index 1e060654bbb..a13bae64da3 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go @@ -167,6 +167,7 @@ func autoConvert_v1_AuthInfo_To_api_AuthInfo(in *AuthInfo, out *api.AuthInfo, s out.Token = in.Token out.TokenFile = in.TokenFile out.Impersonate = in.Impersonate + out.ImpersonateUID = in.ImpersonateUID out.ImpersonateGroups = *(*[]string)(unsafe.Pointer(&in.ImpersonateGroups)) out.ImpersonateUserExtra = *(*map[string][]string)(unsafe.Pointer(&in.ImpersonateUserExtra)) out.Username = in.Username @@ -201,6 +202,7 @@ func autoConvert_api_AuthInfo_To_v1_AuthInfo(in *api.AuthInfo, out *AuthInfo, s out.Token = in.Token out.TokenFile = in.TokenFile out.Impersonate = in.Impersonate + out.ImpersonateUID = in.ImpersonateUID out.ImpersonateGroups = *(*[]string)(unsafe.Pointer(&in.ImpersonateGroups)) out.ImpersonateUserExtra = *(*map[string][]string)(unsafe.Pointer(&in.ImpersonateUserExtra)) out.Username = in.Username diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go b/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go index 0a905490c90..cc37c9fbf6b 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go @@ -181,6 +181,7 @@ func (config *DirectClientConfig) ClientConfig() (*restclient.Config, error) { if len(configAuthInfo.Impersonate) > 0 { clientConfig.Impersonate = restclient.ImpersonationConfig{ UserName: configAuthInfo.Impersonate, + UID: configAuthInfo.ImpersonateUID, Groups: configAuthInfo.ImpersonateGroups, Extra: configAuthInfo.ImpersonateUserExtra, } @@ -255,6 +256,7 @@ func (config *DirectClientConfig) getUserIdentificationPartialConfig(configAuthI if len(configAuthInfo.Impersonate) > 0 { mergedConfig.Impersonate = restclient.ImpersonationConfig{ UserName: configAuthInfo.Impersonate, + UID: configAuthInfo.ImpersonateUID, Groups: configAuthInfo.ImpersonateGroups, Extra: configAuthInfo.ImpersonateUserExtra, } diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go index a770dd61cc3..6af3245863b 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go @@ -217,6 +217,43 @@ func TestTLSServerNameClearsWhenServerNameSet(t *testing.T) { matchStringArg("", actualCfg.ServerName, t) } +func TestFullImpersonateConfig(t *testing.T) { + config := createValidTestConfig() + config.Clusters["clean"] = &clientcmdapi.Cluster{ + Server: "https://localhost:8443", + } + config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{ + Impersonate: "alice", + ImpersonateUID: "abc123", + ImpersonateGroups: []string{"group-1"}, + ImpersonateUserExtra: map[string][]string{"some-key": {"some-value"}}, + } + config.Contexts["clean"] = &clientcmdapi.Context{ + Cluster: "clean", + AuthInfo: "clean", + } + config.CurrentContext = "clean" + + clientBuilder := NewNonInteractiveClientConfig(*config, "clean", &ConfigOverrides{ + ClusterInfo: clientcmdapi.Cluster{ + Server: "http://something", + }, + }, nil) + + actualCfg, err := clientBuilder.ClientConfig() + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + matchStringArg("alice", actualCfg.Impersonate.UserName, t) + matchStringArg("abc123", actualCfg.Impersonate.UID, t) + matchIntArg(1, len(actualCfg.Impersonate.Groups), t) + matchStringArg("group-1", actualCfg.Impersonate.Groups[0], t) + matchIntArg(1, len(actualCfg.Impersonate.Extra), t) + matchIntArg(1, len(actualCfg.Impersonate.Extra["some-key"]), t) + matchStringArg("some-value", actualCfg.Impersonate.Extra["some-key"][0], t) +} + func TestMergeContext(t *testing.T) { const namespace = "overridden-namespace" @@ -808,6 +845,12 @@ func matchByteArg(expected, got []byte, t *testing.T) { } } +func matchIntArg(expected, got int, t *testing.T) { + if expected != got { + t.Errorf("Expected %d, got %d", expected, got) + } +} + func TestNamespaceOverride(t *testing.T) { config := &DirectClientConfig{ overrides: &ConfigOverrides{ diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/overrides.go b/staging/src/k8s.io/client-go/tools/clientcmd/overrides.go index 95cba0fac27..ff643cc13da 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/overrides.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/overrides.go @@ -53,6 +53,7 @@ type AuthOverrideFlags struct { ClientKey FlagInfo Token FlagInfo Impersonate FlagInfo + ImpersonateUID FlagInfo ImpersonateGroups FlagInfo Username FlagInfo Password FlagInfo @@ -154,6 +155,7 @@ const ( FlagEmbedCerts = "embed-certs" FlagBearerToken = "token" FlagImpersonate = "as" + FlagImpersonateUID = "as-uid" FlagImpersonateGroup = "as-group" FlagUsername = "username" FlagPassword = "password" @@ -179,6 +181,7 @@ func RecommendedAuthOverrideFlags(prefix string) AuthOverrideFlags { ClientKey: FlagInfo{prefix + FlagKeyFile, "", "", "Path to a client key file for TLS"}, Token: FlagInfo{prefix + FlagBearerToken, "", "", "Bearer token for authentication to the API server"}, Impersonate: FlagInfo{prefix + FlagImpersonate, "", "", "Username to impersonate for the operation"}, + ImpersonateUID: FlagInfo{prefix + FlagImpersonateUID, "", "", "UID to impersonate for the operation"}, ImpersonateGroups: FlagInfo{prefix + FlagImpersonateGroup, "", "", "Group to impersonate for the operation, this flag can be repeated to specify multiple groups."}, Username: FlagInfo{prefix + FlagUsername, "", "", "Username for basic authentication to the API server"}, Password: FlagInfo{prefix + FlagPassword, "", "", "Password for basic authentication to the API server"}, @@ -219,6 +222,7 @@ func BindAuthInfoFlags(authInfo *clientcmdapi.AuthInfo, flags *pflag.FlagSet, fl flagNames.ClientKey.BindStringFlag(flags, &authInfo.ClientKey).AddSecretAnnotation(flags) flagNames.Token.BindStringFlag(flags, &authInfo.Token).AddSecretAnnotation(flags) flagNames.Impersonate.BindStringFlag(flags, &authInfo.Impersonate).AddSecretAnnotation(flags) + flagNames.ImpersonateUID.BindStringFlag(flags, &authInfo.ImpersonateUID).AddSecretAnnotation(flags) flagNames.ImpersonateGroups.BindStringArrayFlag(flags, &authInfo.ImpersonateGroups).AddSecretAnnotation(flags) flagNames.Username.BindStringFlag(flags, &authInfo.Username).AddSecretAnnotation(flags) flagNames.Password.BindStringFlag(flags, &authInfo.Password).AddSecretAnnotation(flags) diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/validation.go b/staging/src/k8s.io/client-go/tools/clientcmd/validation.go index 10c3c6d5a00..2ae1eb706af 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/validation.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/validation.go @@ -323,9 +323,9 @@ func validateAuthInfo(authInfoName string, authInfo clientcmdapi.AuthInfo) []err validationErrors = append(validationErrors, fmt.Errorf("more than one authentication method found for %v; found %v, only one is allowed", authInfoName, methods)) } - // ImpersonateGroups or ImpersonateUserExtra should be requested with a user - if (len(authInfo.ImpersonateGroups) > 0 || len(authInfo.ImpersonateUserExtra) > 0) && (len(authInfo.Impersonate) == 0) { - validationErrors = append(validationErrors, fmt.Errorf("requesting groups or user-extra for %v without impersonating a user", authInfoName)) + // ImpersonateUID, ImpersonateGroups or ImpersonateUserExtra should be requested with a user + if (len(authInfo.ImpersonateUID) > 0 || len(authInfo.ImpersonateGroups) > 0 || len(authInfo.ImpersonateUserExtra) > 0) && (len(authInfo.Impersonate) == 0) { + validationErrors = append(validationErrors, fmt.Errorf("requesting uid, groups or user-extra for %v without impersonating a user", authInfoName)) } return validationErrors } diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/validation_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/validation_test.go index 676d1989751..bf72c6c098f 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/validation_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/validation_test.go @@ -508,6 +508,78 @@ func TestValidateAuthInfoExecInteractiveModeInvalid(t *testing.T) { test.testConfig(t) } +func TestValidateAuthInfoImpersonateUser(t *testing.T) { + config := clientcmdapi.NewConfig() + config.AuthInfos["user"] = &clientcmdapi.AuthInfo{ + Impersonate: "user", + } + test := configValidationTest{ + config: config, + } + test.testAuthInfo("user", t) + test.testConfig(t) +} + +func TestValidateAuthInfoImpersonateEverything(t *testing.T) { + config := clientcmdapi.NewConfig() + config.AuthInfos["user"] = &clientcmdapi.AuthInfo{ + Impersonate: "user", + ImpersonateUID: "abc123", + ImpersonateGroups: []string{"group-1", "group-2"}, + ImpersonateUserExtra: map[string][]string{"key": {"val1", "val2"}}, + } + test := configValidationTest{ + config: config, + } + test.testAuthInfo("user", t) + test.testConfig(t) +} + +func TestValidateAuthInfoImpersonateGroupsWithoutUserInvalid(t *testing.T) { + config := clientcmdapi.NewConfig() + config.AuthInfos["user"] = &clientcmdapi.AuthInfo{ + ImpersonateGroups: []string{"group-1", "group-2"}, + } + test := configValidationTest{ + config: config, + expectedErrorSubstring: []string{ + `requesting uid, groups or user-extra for user without impersonating a user`, + }, + } + test.testAuthInfo("user", t) + test.testConfig(t) +} + +func TestValidateAuthInfoImpersonateExtraWithoutUserInvalid(t *testing.T) { + config := clientcmdapi.NewConfig() + config.AuthInfos["user"] = &clientcmdapi.AuthInfo{ + ImpersonateUserExtra: map[string][]string{"key": {"val1", "val2"}}, + } + test := configValidationTest{ + config: config, + expectedErrorSubstring: []string{ + `requesting uid, groups or user-extra for user without impersonating a user`, + }, + } + test.testAuthInfo("user", t) + test.testConfig(t) +} + +func TestValidateAuthInfoImpersonateUIDWithoutUserInvalid(t *testing.T) { + config := clientcmdapi.NewConfig() + config.AuthInfos["user"] = &clientcmdapi.AuthInfo{ + ImpersonateUID: "abc123", + } + test := configValidationTest{ + config: config, + expectedErrorSubstring: []string{ + `requesting uid, groups or user-extra for user without impersonating a user`, + }, + } + test.testAuthInfo("user", t) + test.testConfig(t) +} + type configValidationTest struct { config *clientcmdapi.Config expectedErrorSubstring []string diff --git a/test/cmd/authorization.sh b/test/cmd/authorization.sh index d907cdb13f2..80c9e3ed482 100755 --- a/test/cmd/authorization.sh +++ b/test/cmd/authorization.sh @@ -51,6 +51,9 @@ run_impersonation_tests() { output_message=$(! kubectl get pods "${kube_flags_with_token[@]:?}" --as-group=foo 2>&1) kube::test::if_has_string "${output_message}" 'without impersonating a user' + output_message=$(! kubectl get pods "${kube_flags_with_token[@]:?}" --as-uid=abc123 2>&1) + kube::test::if_has_string "${output_message}" 'without impersonating a user' + if kube::test::if_supports_resource "${csr:?}" ; then # --as kubectl create -f hack/testdata/csr.yml "${kube_flags_with_token[@]:?}" --as=user1 @@ -63,6 +66,13 @@ run_impersonation_tests() { kube::test::get_object_assert 'csr/foo' '{{len .spec.groups}}' '4' kube::test::get_object_assert 'csr/foo' '{{range .spec.groups}}{{.}} {{end}}' 'group2 group1 ,,,chameleon system:authenticated ' kubectl delete -f hack/testdata/csr.yml "${kube_flags_with_token[@]:?}" + + # --as-uid + kubectl create -f hack/testdata/csr.yml "${kube_flags_with_token[@]:?}" --as=user1 --as-uid=abc123 + kube::test::get_object_assert 'csr/foo' '{{.spec.username}}' 'user1' + kube::test::get_object_assert 'csr/foo' '{{.spec.uid}}' 'abc123' + kubectl delete -f hack/testdata/csr.yml "${kube_flags_with_token[@]:?}" + fi set +o nounset diff --git a/test/cmd/kubeconfig.sh b/test/cmd/kubeconfig.sh index d70553fd823..fd8533fbbf0 100755 --- a/test/cmd/kubeconfig.sh +++ b/test/cmd/kubeconfig.sh @@ -249,6 +249,53 @@ run_client_config_tests() { output_message=$(! kubectl get pod --kubeconfig=missing-config 2>&1) kube::test::if_has_string "${output_message}" 'no such file or directory' + set +o nounset + set +o errexit +} + +run_kubeconfig_impersonate_tests() { + set -o nounset + set -o errexit + + kube::log::status "Testing config with impersonation" + + # copy the existing kubeconfig over and add a new user entry for the admin user impersonating a different user. + kubectl config view --raw > "${TMPDIR:-/tmp}"/impersonateconfig.yaml + cat << EOF >> "${TMPDIR:-/tmp}"/impersonateconfig.yaml +users: +- name: admin-as-userb + user: + # token defined in hack/testdata/auth-tokens.csv + token: admin-token + # impersonated user + as: userb + as-uid: abc123 + as-groups: + - group2 + - group1 + as-user-extra: + foo: + - bar + - baz +- name: as-uid-without-as + user: + # token defined in hack/testdata/auth-tokens.csv + token: admin-token + # impersonated uid + as-uid: abc123 +EOF + + kubectl create -f hack/testdata/csr.yml --kubeconfig "${TMPDIR:-/tmp}"/impersonateconfig.yaml --user admin-as-userb + kube::test::get_object_assert 'csr/foo' '{{.spec.username}}' 'userb' + kube::test::get_object_assert 'csr/foo' '{{.spec.uid}}' 'abc123' + kube::test::get_object_assert 'csr/foo' '{{range .spec.groups}}{{.}} {{end}}' 'group2 group1 system:authenticated ' + kube::test::get_object_assert 'csr/foo' '{{len .spec.extra}}' '1' + kube::test::get_object_assert 'csr/foo' '{{range .spec.extra.foo}}{{.}} {{end}}' 'bar baz ' + kubectl delete -f hack/testdata/csr.yml + + output_message=$(! kubectl get pods --kubeconfig "${TMPDIR:-/tmp}"/impersonateconfig.yaml --user as-uid-without-as 2>&1) + kube::test::if_has_string "${output_message}" 'without impersonating a user' + set +o nounset set +o errexit } \ No newline at end of file