From 344babe4353e1828dba61a7f1933a9e7eb93eaef Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Fri, 16 Feb 2024 09:23:30 -0300 Subject: [PATCH 01/11] feat: support issuing cluster credential from external command Signed-off-by: Claudio Netto --- .../cluster/v1alpha1/clustergateway_types.go | 2 + .../v1alpha1/clustergateway_types_secret.go | 66 ++++++++++- .../clustergateway_types_secret_test.go | 107 +++++++++++++++-- pkg/util/exec/exec.go | 109 ++++++++++++++++++ 4 files changed, 273 insertions(+), 11 deletions(-) create mode 100644 pkg/util/exec/exec.go diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types.go b/pkg/apis/cluster/v1alpha1/clustergateway_types.go index fcac4eeb..4ee89ebd 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types.go @@ -79,6 +79,8 @@ const ( // CredentialTypeX509Certificate means the cluster is accessible via // X509 certificate and key. CredentialTypeX509Certificate CredentialType = "X509Certificate" + // CredentialTypeDynamicServiceAccountToken ... + CredentialTypePodIdentity CredentialType = "PodIdentity" ) type ClusterEndpointType string diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go index 7622306d..be2632c0 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go @@ -2,17 +2,22 @@ package v1alpha1 import ( "context" + "encoding/json" "fmt" "strconv" "strings" "k8s.io/apimachinery/pkg/util/yaml" + "k8s.io/apiserver/pkg/registry/rest" + "k8s.io/client-go/pkg/apis/clientauthentication" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/utils/pointer" "github.com/oam-dev/cluster-gateway/pkg/common" "github.com/oam-dev/cluster-gateway/pkg/config" "github.com/oam-dev/cluster-gateway/pkg/featuregates" "github.com/oam-dev/cluster-gateway/pkg/options" + "github.com/oam-dev/cluster-gateway/pkg/util/exec" "github.com/oam-dev/cluster-gateway/pkg/util/singleton" "github.com/pkg/errors" @@ -22,7 +27,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apiserver/pkg/registry/rest" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" clusterv1 "open-cluster-management.io/api/cluster/v1" @@ -176,11 +180,11 @@ func getEndpointFromSecret(secret *v1.Secret) ([]byte, string, error) { func convert(caData []byte, apiServerEndpoint string, insecure bool, secret *v1.Secret) (*ClusterGateway, error) { c := &ClusterGateway{ ObjectMeta: metav1.ObjectMeta{ - Name: secret.Name, + Name: secret.Name, + CreationTimestamp: secret.CreationTimestamp, }, Spec: ClusterGatewaySpec{ - Provider: "", - Access: ClusterAccess{}, + Access: ClusterAccess{}, }, } @@ -242,11 +246,21 @@ func convert(caData []byte, apiServerEndpoint string, insecure bool, secret *v1. PrivateKey: secret.Data[v1.TLSPrivateKeyKey], }, } + case CredentialTypeServiceAccountToken: c.Spec.Access.Credential = &ClusterAccessCredential{ Type: CredentialTypeServiceAccountToken, ServiceAccountToken: string(secret.Data[v1.ServiceAccountTokenKey]), } + + case CredentialTypePodIdentity: + credential, err := buildCredentialFromExecConfig(secret, c.Spec.Access.Endpoint) + if err != nil { + return nil, fmt.Errorf("failed to issue credential from external command: %s", err) + } + + c.Spec.Access.Credential = credential + default: return nil, fmt.Errorf("unrecognized secret credential type %v", credentialType) } @@ -278,3 +292,47 @@ func convert(caData []byte, apiServerEndpoint string, insecure bool, secret *v1. return c, nil } + +func buildCredentialFromExecConfig(secret *v1.Secret, ep *ClusterEndpoint) (*ClusterAccessCredential, error) { + execConfigRaw := secret.Data["exec"] + if len(execConfigRaw) == 0 { + return nil, errors.New("missing secret data key: exec") + } + + var ec clientcmdapi.ExecConfig + if err := json.Unmarshal(execConfigRaw, &ec); err != nil { + return nil, fmt.Errorf("failed to decode exec config from secret data: %v", err) + } + + cluster := &clientauthentication.Cluster{ + Server: ep.Const.Address, + CertificateAuthorityData: ep.Const.CABundle, + InsecureSkipTLSVerify: pointer.BoolDeref(ep.Const.Insecure, false), + ProxyURL: pointer.StringDeref(ep.Const.ProxyURL, ""), + } + + cred, err := exec.GetToken(&ec, cluster) + if err != nil { + return nil, err + } + + if token := cred.Status.Token; len(token) > 0 { + return &ClusterAccessCredential{ + Type: CredentialTypeServiceAccountToken, + ServiceAccountToken: token, + }, nil + } + + if cert, key := cred.Status.ClientCertificateData, cred.Status.ClientKeyData; len(cert) > 0 && len(key) > 0 { + return &ClusterAccessCredential{ + Type: CredentialTypeX509Certificate, + X509: &X509{ + Certificate: []byte(cert), + PrivateKey: []byte(key), + }, + }, nil + + } + + return nil, fmt.Errorf("no credential type available") +} diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go index 0c489350..bd308008 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go @@ -27,13 +27,29 @@ import ( ) var ( - testNamespace = "foo" - testName = "bar" - testCAData = "caData" - testCertData = "certData" - testKeyData = "keyData" - testToken = "token" - testEndpoint = "https://localhost:443" + testNamespace = "foo" + testName = "bar" + testCAData = "caData" + testCertData = "certData" + testKeyData = "keyData" + testToken = "token" + testEndpoint = "https://localhost:443" + testExecConfigForToken = `{ + "apiVersion": "client.authentication.k8s.io/v1beta1", + "kind": "ExecConfig", + "command": "echo", + "args": [ + "{\"apiVersion\": \"client.authentication.k8s.io/v1beta1\", \"kind\": \"ExecCredential\", \"status\": {\"token\": \"token\"}}" + ] +}` + testExecConfigForX509 = `{ + "apiVersion": "client.authentication.k8s.io/v1beta1", + "kind": "ExecConfig", + "command": "echo", + "args": [ + "{\"apiVersion\": \"client.authentication.k8s.io/v1beta1\", \"kind\": \"ExecCredential\", \"status\": {\"clientCertificateData\": \"certData\", \"clientKeyData\": \"keyData\"}}" + ] +}` ) func TestConvertSecretToGateway(t *testing.T) { @@ -260,6 +276,83 @@ func TestConvertSecretToGateway(t *testing.T) { }, }, }, + { + name: "service account token issued from external command", + inputSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + Namespace: testNamespace, + Labels: map[string]string{ + common.LabelKeyClusterCredentialType: string(CredentialTypePodIdentity), + }, + }, + Data: map[string][]byte{ + "endpoint": []byte(testEndpoint), + "ca.crt": []byte(testCAData), + "exec": []byte(testExecConfigForToken), + }, + }, + expected: &ClusterGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + }, + Spec: ClusterGatewaySpec{ + Access: ClusterAccess{ + Credential: &ClusterAccessCredential{ + Type: CredentialTypeServiceAccountToken, + ServiceAccountToken: testToken, + }, + Endpoint: &ClusterEndpoint{ + Type: ClusterEndpointTypeConst, + Const: &ClusterEndpointConst{ + CABundle: []byte(testCAData), + Address: testEndpoint, + }, + }, + }, + }, + }, + }, + { + name: "x509 cert-key pair issued from external command", + inputSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + Namespace: testNamespace, + Labels: map[string]string{ + common.LabelKeyClusterCredentialType: string(CredentialTypePodIdentity), + }, + }, + Data: map[string][]byte{ + "endpoint": []byte(testEndpoint), + "ca.crt": []byte(testCAData), + "exec": []byte(testExecConfigForX509), + }, + }, + expected: &ClusterGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + }, + Spec: ClusterGatewaySpec{ + Access: ClusterAccess{ + Credential: &ClusterAccessCredential{ + Type: CredentialTypeX509Certificate, + X509: &X509{ + Certificate: []byte(testCertData), + PrivateKey: []byte(testKeyData), + }, + }, + Endpoint: &ClusterEndpoint{ + Type: ClusterEndpointTypeConst, + Const: &ClusterEndpointConst{ + CABundle: []byte(testCAData), + Address: testEndpoint, + }, + }, + }, + }, + }, + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/pkg/util/exec/exec.go b/pkg/util/exec/exec.go new file mode 100644 index 00000000..3cfa4b65 --- /dev/null +++ b/pkg/util/exec/exec.go @@ -0,0 +1,109 @@ +package exec + +import ( + "bytes" + "fmt" + "os" + "os/exec" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer" + + "k8s.io/client-go/pkg/apis/clientauthentication" + "k8s.io/client-go/pkg/apis/clientauthentication/install" + clientauthenticationv1 "k8s.io/client-go/pkg/apis/clientauthentication/v1" + clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" +) + +var ( + scheme = runtime.NewScheme() + + codecs = serializer.NewCodecFactory(scheme) + + apiVersions = map[string]schema.GroupVersion{ + clientauthenticationv1beta1.SchemeGroupVersion.String(): clientauthenticationv1beta1.SchemeGroupVersion, + clientauthenticationv1.SchemeGroupVersion.String(): clientauthenticationv1.SchemeGroupVersion, + } +) + +func init() { + install.Install(scheme) +} + +func GetToken(ec *clientcmdapi.ExecConfig, cluster *clientauthentication.Cluster) (*clientauthentication.ExecCredential, error) { + cmd := exec.Command(ec.Command, ec.Args...) + cmd.Env = os.Environ() + + for _, env := range ec.Env { + cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", env.Name, env.Value)) + } + + var stderr, stdout bytes.Buffer + cmd.Stderr = &stderr + cmd.Stdout = &stdout + + if err := cmd.Run(); err != nil { + return nil, wrapCmdRunErrorLocked(cmd, err) + } + + ecgv, err := schema.ParseGroupVersion(ec.APIVersion) + if err != nil { + return nil, fmt.Errorf("failed to parse exec config API version: %v", err) + } + + cred := &clientauthentication.ExecCredential{ + TypeMeta: metav1.TypeMeta{ + APIVersion: ec.APIVersion, + Kind: "ExecCredential", + }, + Spec: clientauthentication.ExecCredentialSpec{ + Interactive: false, + Cluster: cluster, + }, + } + + gv, ok := apiVersions[ec.APIVersion] + if !ok { + return nil, fmt.Errorf("exec plugin: invalid apiVersion %q", ec.APIVersion) + } + + _, gvk, err := codecs.UniversalDecoder(gv).Decode(stdout.Bytes(), nil, cred) + if err != nil { + return nil, fmt.Errorf("decoding stdout: %v", err) + } + + if gvk.Group != ecgv.Group || gvk.Version != ecgv.Version { + return nil, fmt.Errorf("exec plugin is configured to use API version %s, plugin returned version %s", ecgv, schema.GroupVersion{Group: gvk.Group, Version: gvk.Version}) + } + + if cred.Status == nil { + return nil, fmt.Errorf("exec plugin didn't return a status field") + } + + if cred.Status.Token == "" && cred.Status.ClientCertificateData == "" && cred.Status.ClientKeyData == "" { + return nil, fmt.Errorf("exec plugin didn't return a token or cert/key pair") + } + + if (cred.Status.ClientCertificateData == "") != (cred.Status.ClientKeyData == "") { + return nil, fmt.Errorf("exec plugin returned only certificate or key, not both") + } + + return cred, nil +} + +func wrapCmdRunErrorLocked(cmd *exec.Cmd, err error) error { + switch err.(type) { + case *exec.Error: // Binary does not exist (see exec.Error). + return fmt.Errorf("exec: executable %s not found", cmd.Path) + + case *exec.ExitError: // Binary execution failed (see exec.Cmd.Run()). + e := err.(*exec.ExitError) + return fmt.Errorf("exec: executable %s failed with exit code %d", cmd.Path, e.ProcessState.ExitCode()) + + default: + return fmt.Errorf("exec: %v", err) + } +} From e05c1fbe36773407f84b20ff0b7eb46ae868f5b3 Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Fri, 16 Feb 2024 11:20:41 -0300 Subject: [PATCH 02/11] refactor: rename the dynamic credential type Signed-off-by: Claudio Netto --- pkg/apis/cluster/v1alpha1/clustergateway_types.go | 6 ++++-- pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go | 2 +- .../cluster/v1alpha1/clustergateway_types_secret_test.go | 8 ++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types.go b/pkg/apis/cluster/v1alpha1/clustergateway_types.go index 4ee89ebd..65e610aa 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types.go @@ -79,8 +79,10 @@ const ( // CredentialTypeX509Certificate means the cluster is accessible via // X509 certificate and key. CredentialTypeX509Certificate CredentialType = "X509Certificate" - // CredentialTypeDynamicServiceAccountToken ... - CredentialTypePodIdentity CredentialType = "PodIdentity" + // CredentialTypeDynamic means that a credential will be issued before + // accessing the cluster. The generated credential can be either a service + // account token or X509 certificate and key. + CredentialTypeDynamic CredentialType = "Dynamic" ) type ClusterEndpointType string diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go index be2632c0..dbd3716b 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go @@ -253,7 +253,7 @@ func convert(caData []byte, apiServerEndpoint string, insecure bool, secret *v1. ServiceAccountToken: string(secret.Data[v1.ServiceAccountTokenKey]), } - case CredentialTypePodIdentity: + case CredentialTypeDynamic: credential, err := buildCredentialFromExecConfig(secret, c.Spec.Access.Endpoint) if err != nil { return nil, fmt.Errorf("failed to issue credential from external command: %s", err) diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go index bd308008..45be3272 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go @@ -277,13 +277,13 @@ func TestConvertSecretToGateway(t *testing.T) { }, }, { - name: "service account token issued from external command", + name: "dynamic service account token issued from external command", inputSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: testName, Namespace: testNamespace, Labels: map[string]string{ - common.LabelKeyClusterCredentialType: string(CredentialTypePodIdentity), + common.LabelKeyClusterCredentialType: string(CredentialTypeDynamic), }, }, Data: map[string][]byte{ @@ -314,13 +314,13 @@ func TestConvertSecretToGateway(t *testing.T) { }, }, { - name: "x509 cert-key pair issued from external command", + name: "dynamic x509 cert-key pair issued from external command", inputSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: testName, Namespace: testNamespace, Labels: map[string]string{ - common.LabelKeyClusterCredentialType: string(CredentialTypePodIdentity), + common.LabelKeyClusterCredentialType: string(CredentialTypeDynamic), }, }, Data: map[string][]byte{ From 92e54670b332cf85af9da9d52a53d0aaed692dd6 Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Fri, 16 Feb 2024 13:45:02 -0300 Subject: [PATCH 03/11] refactor: covering dynamic credentials w/ unit tests Signed-off-by: Claudio Netto --- .../v1alpha1/clustergateway_types_secret.go | 2 +- .../clustergateway_types_secret_test.go | 177 ++++++++++++++++++ pkg/util/exec/exec.go | 20 +- 3 files changed, 193 insertions(+), 6 deletions(-) diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go index dbd3716b..18107bb7 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go @@ -301,7 +301,7 @@ func buildCredentialFromExecConfig(secret *v1.Secret, ep *ClusterEndpoint) (*Clu var ec clientcmdapi.ExecConfig if err := json.Unmarshal(execConfigRaw, &ec); err != nil { - return nil, fmt.Errorf("failed to decode exec config from secret data: %v", err) + return nil, fmt.Errorf("failed to decode exec config JSON from secret data: %v", err) } cluster := &clientauthentication.Cluster{ diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go index 45be3272..23acb3e4 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go @@ -617,3 +617,180 @@ func TestListHybridClusterGateway(t *testing.T) { } assert.Equal(t, expectedNames, actualNames) } + +func TestBuildCredentialFromExecConfig(t *testing.T) { + cases := []struct { + name string + secret func(s *corev1.Secret) *corev1.Secret + cluster func(ce *ClusterEndpoint) *ClusterEndpoint + expectedError string + expected *ClusterAccessCredential + }{ + { + name: "missing exec config", + expectedError: "missing secret data key: exec", + }, + + { + name: "invalid exec config format", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte("some invalid exec config") + return s + }, + expectedError: "failed to decode exec config JSON from secret data: invalid character 's' looking for beginning of value", + }, + + { + name: "missing command property within exec config", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte(`{}`) + return s + }, + expectedError: "missing command key in ExecConfig object", + }, + + { + name: "failed to run external command: command not found", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte(`{"command": "/path/to/command/not/found"}`) + return s + }, + expectedError: "exec: executable /path/to/command/not/found not found", + }, + + { + name: "failed to run external command: finished with non-zero exit code", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte(`{"command": "false"}`) + return s + }, + expectedError: "exec: executable /usr/bin/false failed with exit code 1", + }, + + { + name: "missing API version in exec config", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte(`{"command": "true"}`) + return s + }, + expectedError: "exec plugin: invalid apiVersion \"\"", + }, + + { + name: "invalid API version in exec config", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte(`{"apiVersion": "example.org/v1", "command": "true"}`) + return s + }, + expectedError: "exec plugin: invalid apiVersion \"example.org/v1\"", + }, + + { + name: "invalid external command output", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte(`{"apiVersion": "client.authentication.k8s.io/v1", "command": "echo", "args": ["invalid output"]}`) + return s + }, + expectedError: "decoding stdout: couldn't get version/kind; json parse error: json: cannot unmarshal string into Go value of type struct { APIVersion string \"json:\\\"apiVersion,omitempty\\\"\"; Kind string \"json:\\\"kind,omitempty\\\"\" }", + }, + + { + name: "API version mismatch between exec config and command output", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte(`{"apiVersion": "client.authentication.k8s.io/v1", "command": "echo", "args": ["{\"apiVersion\": \"client.authentication.k8s.io/v1beta1\"}"]}`) + return s + }, + expectedError: "exec plugin is configured to use API version client.authentication.k8s.io/v1, plugin returned version client.authentication.k8s.io/v1beta1", + }, + + { + name: "missing status property on external command output", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte(`{"apiVersion": "client.authentication.k8s.io/v1", "command": "echo", "args": ["{\"apiVersion\": \"client.authentication.k8s.io/v1\"}"]}`) + return s + }, + expectedError: "exec plugin didn't return a status field", + }, + + { + name: "missing any auth credential on status", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte(`{"apiVersion": "client.authentication.k8s.io/v1", "command": "echo", "args": ["{\"apiVersion\": \"client.authentication.k8s.io/v1\", \"status\": {}}"]}`) + return s + }, + expectedError: "exec plugin didn't return a token or cert/key pair", + }, + + { + name: "has cert but no private key", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte(`{"apiVersion": "client.authentication.k8s.io/v1", "command": "echo", "args": ["{\"apiVersion\": \"client.authentication.k8s.io/v1\", \"status\": {\"clientCertificateData\": \"certData\"}}"]}`) + return s + }, + expectedError: "exec plugin returned only certificate or key, not both", + }, + + { + name: "returns successfully a service account token", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte(`{"apiVersion": "client.authentication.k8s.io/v1", "command": "echo", "args": ["{\"apiVersion\": \"client.authentication.k8s.io/v1\", \"status\": {\"token\": \"token\"}}"]}`) + return s + }, + expected: &ClusterAccessCredential{ + Type: CredentialTypeServiceAccountToken, + ServiceAccountToken: testToken, + }, + }, + + { + name: "returns successfully a X509 client certificate", + secret: func(s *corev1.Secret) *corev1.Secret { + s.Data["exec"] = []byte(`{"apiVersion": "client.authentication.k8s.io/v1", "command": "echo", "args": ["{\"apiVersion\": \"client.authentication.k8s.io/v1\", \"status\": {\"clientCertificateData\": \"certData\", \"clientKeyData\": \"keyData\"}}"]}`) + return s + }, + expected: &ClusterAccessCredential{ + Type: CredentialTypeX509Certificate, + X509: &X509{ + Certificate: []byte(testCertData), + PrivateKey: []byte(testKeyData), + }, + }, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + Namespace: testNamespace, + }, + Data: map[string][]byte{}, + } + if tt.secret != nil { + secret = tt.secret(secret) + } + + cluster := &ClusterEndpoint{ + Type: ClusterEndpointTypeConst, + Const: &ClusterEndpointConst{ + Address: testEndpoint, + CABundle: []byte(testCAData), + }, + } + if tt.cluster != nil { + cluster = tt.cluster(cluster) + } + + got, err := buildCredentialFromExecConfig(secret, cluster) + if tt.expectedError != "" { + assert.Error(t, err) + assert.EqualError(t, err, tt.expectedError) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.expected, got) + }) + } +} diff --git a/pkg/util/exec/exec.go b/pkg/util/exec/exec.go index 3cfa4b65..04ac5be5 100644 --- a/pkg/util/exec/exec.go +++ b/pkg/util/exec/exec.go @@ -2,6 +2,7 @@ package exec import ( "bytes" + "errors" "fmt" "os" "os/exec" @@ -34,7 +35,16 @@ func init() { } func GetToken(ec *clientcmdapi.ExecConfig, cluster *clientauthentication.Cluster) (*clientauthentication.ExecCredential, error) { - cmd := exec.Command(ec.Command, ec.Args...) + if ec.Command == "" { + return nil, errors.New("missing command key in ExecConfig object") + } + + command, err := exec.LookPath(ec.Command) + if err != nil { + return nil, unwrapExecCommandError(ec.Command, err) + } + + cmd := exec.Command(command, ec.Args...) cmd.Env = os.Environ() for _, env := range ec.Env { @@ -46,7 +56,7 @@ func GetToken(ec *clientcmdapi.ExecConfig, cluster *clientauthentication.Cluster cmd.Stdout = &stdout if err := cmd.Run(); err != nil { - return nil, wrapCmdRunErrorLocked(cmd, err) + return nil, unwrapExecCommandError(command, err) } ecgv, err := schema.ParseGroupVersion(ec.APIVersion) @@ -94,14 +104,14 @@ func GetToken(ec *clientcmdapi.ExecConfig, cluster *clientauthentication.Cluster return cred, nil } -func wrapCmdRunErrorLocked(cmd *exec.Cmd, err error) error { +func unwrapExecCommandError(path string, err error) error { switch err.(type) { case *exec.Error: // Binary does not exist (see exec.Error). - return fmt.Errorf("exec: executable %s not found", cmd.Path) + return fmt.Errorf("exec: executable %s not found", path) case *exec.ExitError: // Binary execution failed (see exec.Cmd.Run()). e := err.(*exec.ExitError) - return fmt.Errorf("exec: executable %s failed with exit code %d", cmd.Path, e.ProcessState.ExitCode()) + return fmt.Errorf("exec: executable %s failed with exit code %d", path, e.ProcessState.ExitCode()) default: return fmt.Errorf("exec: %v", err) From 9971b1698a94d7b6aa671705b0f2d42f57da5698 Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Tue, 20 Feb 2024 11:59:28 -0300 Subject: [PATCH 04/11] docs: adding an example of secret using Dynamic cluster credential Signed-off-by: Claudio Netto --- docs/local-run.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/docs/local-run.md b/docs/local-run.md index 0330ba7d..01dd1200 100644 --- a/docs/local-run.md +++ b/docs/local-run.md @@ -167,6 +167,21 @@ data: token: "..." # working jwt token ``` +2.3. (Alternatively) Create a secret containing an exec config to dynamically fetch the cluster credential from an external command: + +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: managed1 + labels: + cluster.core.oam.dev/cluster-credential-type: Dynamic +type: Opaque # <--- Has to be opaque +data: + endpoint: "..." # ditto + exec: "..." # an exec config in JSON format; see ExecConfig (https://github.com/kubernetes/kubernetes/blob/2016fab3085562b4132e6d3774b6ded5ba9939fd/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go#L206, https://kubernetes.io/docs/reference/access-authn-authz/authentication/#configuration) +``` + 3. Proxy to cluster `managed1`'s `/healthz` endpoint ```shell @@ -195,4 +210,4 @@ KUBECONFIG=/tmp/hub-managed1.kubeconfig kubectl get ns ```shell $ kind delete cluster --name tmp -``` \ No newline at end of file +``` From 9f09606d691bc6d2424cd8830c379b8925c5141e Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Tue, 20 Feb 2024 12:06:51 -0300 Subject: [PATCH 05/11] test: covering code branch of failed to build cluster credentials Signed-off-by: Claudio Netto --- .../clustergateway_types_secret_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go index 23acb3e4..648955a5 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go @@ -353,6 +353,24 @@ func TestConvertSecretToGateway(t *testing.T) { }, }, }, + { + name: "failed to fetch cluster credential from dynamic auth mode", + inputSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + Namespace: testNamespace, + Labels: map[string]string{ + common.LabelKeyClusterCredentialType: string(CredentialTypeDynamic), + }, + }, + Data: map[string][]byte{ + "endpoint": []byte(testEndpoint), + "ca.crt": []byte(testCAData), + "exec": []byte("invalid exec config format"), + }, + }, + expectedFailure: true, + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { From 02bf3586d66a9e406864fcebfaa9228511cb9536 Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Tue, 20 Feb 2024 14:48:36 -0300 Subject: [PATCH 06/11] refactor: return ClusterGateway w/ credential type Dynamic Signed-off-by: Claudio Netto --- .../cluster/v1alpha1/clustergateway_types_secret.go | 4 ++-- .../v1alpha1/clustergateway_types_secret_test.go | 8 ++++---- pkg/apis/cluster/v1alpha1/transport.go | 11 +++++++++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go index 18107bb7..98f5873f 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go @@ -318,14 +318,14 @@ func buildCredentialFromExecConfig(secret *v1.Secret, ep *ClusterEndpoint) (*Clu if token := cred.Status.Token; len(token) > 0 { return &ClusterAccessCredential{ - Type: CredentialTypeServiceAccountToken, + Type: CredentialTypeDynamic, ServiceAccountToken: token, }, nil } if cert, key := cred.Status.ClientCertificateData, cred.Status.ClientKeyData; len(cert) > 0 && len(key) > 0 { return &ClusterAccessCredential{ - Type: CredentialTypeX509Certificate, + Type: CredentialTypeDynamic, X509: &X509{ Certificate: []byte(cert), PrivateKey: []byte(key), diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go index 648955a5..357ac2e9 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go @@ -299,7 +299,7 @@ func TestConvertSecretToGateway(t *testing.T) { Spec: ClusterGatewaySpec{ Access: ClusterAccess{ Credential: &ClusterAccessCredential{ - Type: CredentialTypeServiceAccountToken, + Type: CredentialTypeDynamic, ServiceAccountToken: testToken, }, Endpoint: &ClusterEndpoint{ @@ -336,7 +336,7 @@ func TestConvertSecretToGateway(t *testing.T) { Spec: ClusterGatewaySpec{ Access: ClusterAccess{ Credential: &ClusterAccessCredential{ - Type: CredentialTypeX509Certificate, + Type: CredentialTypeDynamic, X509: &X509{ Certificate: []byte(testCertData), PrivateKey: []byte(testKeyData), @@ -755,7 +755,7 @@ func TestBuildCredentialFromExecConfig(t *testing.T) { return s }, expected: &ClusterAccessCredential{ - Type: CredentialTypeServiceAccountToken, + Type: CredentialTypeDynamic, ServiceAccountToken: testToken, }, }, @@ -767,7 +767,7 @@ func TestBuildCredentialFromExecConfig(t *testing.T) { return s }, expected: &ClusterAccessCredential{ - Type: CredentialTypeX509Certificate, + Type: CredentialTypeDynamic, X509: &X509{ Certificate: []byte(testCertData), PrivateKey: []byte(testKeyData), diff --git a/pkg/apis/cluster/v1alpha1/transport.go b/pkg/apis/cluster/v1alpha1/transport.go index f15698fd..3a1f399a 100644 --- a/pkg/apis/cluster/v1alpha1/transport.go +++ b/pkg/apis/cluster/v1alpha1/transport.go @@ -94,8 +94,19 @@ func NewConfigFromCluster(ctx context.Context, c *ClusterGateway) (*restclient.C } // setting up credentials switch c.Spec.Access.Credential.Type { + case CredentialTypeDynamic: + if token := c.Spec.Access.Credential.ServiceAccountToken; token != "" { + cfg.BearerToken = token + } + + if c.Spec.Access.Credential.X509 != nil && len(c.Spec.Access.Credential.X509.Certificate) > 0 && len(c.Spec.Access.Credential.X509.PrivateKey) > 0 { + cfg.CertData = c.Spec.Access.Credential.X509.Certificate + cfg.KeyData = c.Spec.Access.Credential.X509.PrivateKey + } + case CredentialTypeServiceAccountToken: cfg.BearerToken = c.Spec.Access.Credential.ServiceAccountToken + case CredentialTypeX509Certificate: cfg.CertData = c.Spec.Access.Credential.X509.Certificate cfg.KeyData = c.Spec.Access.Credential.X509.PrivateKey From def47b262fc34ca31a9e808210ff23867d962116 Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Tue, 27 Feb 2024 21:14:22 -0300 Subject: [PATCH 07/11] feat: store/fetch credential from in-memory cache Signed-off-by: Claudio Netto --- .../v1alpha1/clustergateway_types_secret.go | 14 +- .../clustergateway_types_secret_test.go | 15 +- pkg/util/exec/exec.go | 51 ++++- pkg/util/exec/exec_test.go | 177 ++++++++++++++++++ 4 files changed, 227 insertions(+), 30 deletions(-) create mode 100644 pkg/util/exec/exec_test.go diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go index 98f5873f..ed88a846 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret.go @@ -9,7 +9,6 @@ import ( "k8s.io/apimachinery/pkg/util/yaml" "k8s.io/apiserver/pkg/registry/rest" - "k8s.io/client-go/pkg/apis/clientauthentication" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/utils/pointer" @@ -254,7 +253,7 @@ func convert(caData []byte, apiServerEndpoint string, insecure bool, secret *v1. } case CredentialTypeDynamic: - credential, err := buildCredentialFromExecConfig(secret, c.Spec.Access.Endpoint) + credential, err := buildCredentialFromExecConfig(secret) if err != nil { return nil, fmt.Errorf("failed to issue credential from external command: %s", err) } @@ -293,7 +292,7 @@ func convert(caData []byte, apiServerEndpoint string, insecure bool, secret *v1. return c, nil } -func buildCredentialFromExecConfig(secret *v1.Secret, ep *ClusterEndpoint) (*ClusterAccessCredential, error) { +func buildCredentialFromExecConfig(secret *v1.Secret) (*ClusterAccessCredential, error) { execConfigRaw := secret.Data["exec"] if len(execConfigRaw) == 0 { return nil, errors.New("missing secret data key: exec") @@ -304,14 +303,7 @@ func buildCredentialFromExecConfig(secret *v1.Secret, ep *ClusterEndpoint) (*Clu return nil, fmt.Errorf("failed to decode exec config JSON from secret data: %v", err) } - cluster := &clientauthentication.Cluster{ - Server: ep.Const.Address, - CertificateAuthorityData: ep.Const.CABundle, - InsecureSkipTLSVerify: pointer.BoolDeref(ep.Const.Insecure, false), - ProxyURL: pointer.StringDeref(ep.Const.ProxyURL, ""), - } - - cred, err := exec.GetToken(&ec, cluster) + cred, err := exec.IssueClusterCredential(secret.Name, &ec) if err != nil { return nil, err } diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go index 357ac2e9..86c8f414 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go @@ -664,7 +664,7 @@ func TestBuildCredentialFromExecConfig(t *testing.T) { s.Data["exec"] = []byte(`{}`) return s }, - expectedError: "missing command key in ExecConfig object", + expectedError: "missing \"command\" property on exec config object", }, { @@ -789,18 +789,7 @@ func TestBuildCredentialFromExecConfig(t *testing.T) { secret = tt.secret(secret) } - cluster := &ClusterEndpoint{ - Type: ClusterEndpointTypeConst, - Const: &ClusterEndpointConst{ - Address: testEndpoint, - CABundle: []byte(testCAData), - }, - } - if tt.cluster != nil { - cluster = tt.cluster(cluster) - } - - got, err := buildCredentialFromExecConfig(secret, cluster) + got, err := buildCredentialFromExecConfig(secret) if tt.expectedError != "" { assert.Error(t, err) assert.EqualError(t, err, tt.expectedError) diff --git a/pkg/util/exec/exec.go b/pkg/util/exec/exec.go index 04ac5be5..9a89f380 100644 --- a/pkg/util/exec/exec.go +++ b/pkg/util/exec/exec.go @@ -6,6 +6,8 @@ import ( "fmt" "os" "os/exec" + "sync" + "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -28,15 +30,55 @@ var ( clientauthenticationv1beta1.SchemeGroupVersion.String(): clientauthenticationv1beta1.SchemeGroupVersion, clientauthenticationv1.SchemeGroupVersion.String(): clientauthenticationv1.SchemeGroupVersion, } + + credentials sync.Map ) func init() { install.Install(scheme) } -func GetToken(ec *clientcmdapi.ExecConfig, cluster *clientauthentication.Cluster) (*clientauthentication.ExecCredential, error) { +func IssueClusterCredential(name string, ec *clientcmdapi.ExecConfig) (*clientauthentication.ExecCredential, error) { + if name == "" { + return nil, errors.New("cluster name not provided") + } + + value, found := credentials.Load(name) + if found { + cred, ok := value.(*clientauthentication.ExecCredential) + if !ok { + return nil, errors.New("failed to convert item in cache to ExecCredential") + } + + now := &metav1.Time{Time: time.Now().Add(time.Minute)} // expires a minute early + + if cred.Status != nil && cred.Status.ExpirationTimestamp.Before(now) { + credentials.Delete(name) + return IssueClusterCredential(name, ec) // credential expired, calling function again + } + + return cred, nil + } + + cred, err := issueClusterCredential(ec) + if err != nil { + return nil, err + } + + if cred.Status != nil && !cred.Status.ExpirationTimestamp.IsZero() { + credentials.Store(name, cred) // storing credential in cache + } + + return cred, nil +} + +func issueClusterCredential(ec *clientcmdapi.ExecConfig) (*clientauthentication.ExecCredential, error) { + if ec == nil { + return nil, errors.New("exec config not provided") + } + if ec.Command == "" { - return nil, errors.New("missing command key in ExecConfig object") + return nil, errors.New("missing \"command\" property on exec config object") } command, err := exec.LookPath(ec.Command) @@ -69,10 +111,7 @@ func GetToken(ec *clientcmdapi.ExecConfig, cluster *clientauthentication.Cluster APIVersion: ec.APIVersion, Kind: "ExecCredential", }, - Spec: clientauthentication.ExecCredentialSpec{ - Interactive: false, - Cluster: cluster, - }, + Spec: clientauthentication.ExecCredentialSpec{}, } gv, ok := apiVersions[ec.APIVersion] diff --git a/pkg/util/exec/exec_test.go b/pkg/util/exec/exec_test.go new file mode 100644 index 00000000..cbe5d3e4 --- /dev/null +++ b/pkg/util/exec/exec_test.go @@ -0,0 +1,177 @@ +package exec + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/pkg/apis/clientauthentication" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" +) + +var ( + testClusterName = "my-cluster" +) + +func TestIssueClusterCredential(t *testing.T) { + t0 := time.Now() + + cases := map[string]struct { + clusterName string + execConfig *clientcmdapi.ExecConfig + expected *clientauthentication.ExecCredential + expectedError string + setup func(t *testing.T) + }{ + "missing cluster name": { + expectedError: "cluster name not provided", + }, + + "missing exec config": { + clusterName: testClusterName, + expectedError: "exec config not provided", + }, + + "missing command on exec config": { + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{}, + expectedError: "missing \"command\" property on exec config object", + }, + + "successfully issuing a token": { + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1", + Command: "echo", + Args: []string{"-n", `{ + "apiVersion": "client.authentication.k8s.io/v1", + "kind": "ExecCredential", + "status": { + "token": "testToken" + } +}`}, + }, + expected: &clientauthentication.ExecCredential{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "client.authentication.k8s.io/v1", + Kind: "ExecCredential", + }, + Status: &clientauthentication.ExecCredentialStatus{ + Token: "testToken", + }, + }, + }, + + "credential HIT from cache": { + setup: func(t *testing.T) { + credentials.Store(testClusterName, &clientauthentication.ExecCredential{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "client.authentication.k8s.io/v1", + Kind: "ExecCredential", + }, + Status: &clientauthentication.ExecCredentialStatus{ + ExpirationTimestamp: buildMetav1Time(t, t0.Add(time.Hour)), + Token: "testToken", + }, + }) + }, + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1", + Command: "should_be_ignored", + }, + expected: &clientauthentication.ExecCredential{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "client.authentication.k8s.io/v1", + Kind: "ExecCredential", + }, + Status: &clientauthentication.ExecCredentialStatus{ + ExpirationTimestamp: buildMetav1Time(t, t0.Add(time.Hour)), + Token: "testToken", + }, + }, + }, + + "credential MISS from cache": { + setup: func(t *testing.T) { + credentials.Store(testClusterName, &clientauthentication.ExecCredential{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "client.authentication.k8s.io/v1", + Kind: "ExecCredential", + }, + Status: &clientauthentication.ExecCredentialStatus{ + ExpirationTimestamp: buildMetav1Time(t, t0), + Token: "oldToken", + }, + }) + }, + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1", + Command: "echo", + Args: []string{ + "-n", + fmt.Sprintf(`{ + "apiVersion": "client.authentication.k8s.io/v1", + "kind": "ExecCredential", + "status": { + "expirationTimestamp": %q, + "token": "newToken" + } +}`, t0.Add(24*time.Hour).UTC().Format(time.RFC3339)), + }, + }, + expected: &clientauthentication.ExecCredential{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "client.authentication.k8s.io/v1", + Kind: "ExecCredential", + }, + Status: &clientauthentication.ExecCredentialStatus{ + ExpirationTimestamp: buildMetav1Time(t, t0.Add(24*time.Hour)), + Token: "newToken", + }, + }, + }, + } + + for name, tt := range cases { + t.Run(name, func(t *testing.T) { + cleanAllCache(t) + + if tt.setup != nil { + tt.setup(t) + } + + cred, err := IssueClusterCredential(tt.clusterName, tt.execConfig) + if tt.expectedError != "" { + assert.Error(t, err) + assert.EqualError(t, err, tt.expectedError) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.expected, cred) + }) + } +} + +func buildMetav1Time(t *testing.T, tm time.Time) *metav1.Time { + t.Helper() + + copied, err := time.Parse(time.RFC3339, tm.Format(time.RFC3339)) + assert.NoError(t, err, "failed to parse time to RFC3339: %s", err) + + return &metav1.Time{Time: copied} +} + +func cleanAllCache(t *testing.T) { + t.Helper() + + credentials.Range(func(key, value any) bool { + credentials.Delete(key) + return true + }) +} From 2be5d1f12e0ae8f0850d3a07c7ad0d08616e4bf5 Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Wed, 28 Feb 2024 12:04:55 -0300 Subject: [PATCH 08/11] test: fixing exec plugin tests Signed-off-by: Claudio Netto --- pkg/util/exec/exec.go | 2 +- pkg/util/exec/exec_test.go | 35 ++++++++++++++++++----------------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/pkg/util/exec/exec.go b/pkg/util/exec/exec.go index 9a89f380..bb8821ed 100644 --- a/pkg/util/exec/exec.go +++ b/pkg/util/exec/exec.go @@ -57,7 +57,7 @@ func IssueClusterCredential(name string, ec *clientcmdapi.ExecConfig) (*clientau return IssueClusterCredential(name, ec) // credential expired, calling function again } - return cred, nil + return cred, nil // credential on cache still valid } cred, err := issueClusterCredential(ec) diff --git a/pkg/util/exec/exec_test.go b/pkg/util/exec/exec_test.go index cbe5d3e4..8823bed5 100644 --- a/pkg/util/exec/exec_test.go +++ b/pkg/util/exec/exec_test.go @@ -1,3 +1,5 @@ +//go:build unix + package exec import ( @@ -41,7 +43,7 @@ func TestIssueClusterCredential(t *testing.T) { expectedError: "missing \"command\" property on exec config object", }, - "successfully issuing a token": { + "MISS credential from cache, should issue a new credential": { clusterName: testClusterName, execConfig: &clientcmdapi.ExecConfig{ APIVersion: "client.authentication.k8s.io/v1", @@ -65,7 +67,7 @@ func TestIssueClusterCredential(t *testing.T) { }, }, - "credential HIT from cache": { + "HIT credential from cache": { setup: func(t *testing.T) { credentials.Store(testClusterName, &clientauthentication.ExecCredential{ TypeMeta: metav1.TypeMeta{ @@ -73,7 +75,7 @@ func TestIssueClusterCredential(t *testing.T) { Kind: "ExecCredential", }, Status: &clientauthentication.ExecCredentialStatus{ - ExpirationTimestamp: buildMetav1Time(t, t0.Add(time.Hour)), + ExpirationTimestamp: &metav1.Time{Time: t0.Add(time.Hour).Local().Truncate(time.Second)}, Token: "testToken", }, }) @@ -89,13 +91,13 @@ func TestIssueClusterCredential(t *testing.T) { Kind: "ExecCredential", }, Status: &clientauthentication.ExecCredentialStatus{ - ExpirationTimestamp: buildMetav1Time(t, t0.Add(time.Hour)), + ExpirationTimestamp: &metav1.Time{Time: t0.Add(time.Hour).Local().Truncate(time.Second)}, Token: "testToken", }, }, }, - "credential MISS from cache": { + "expired credential on cache, should issue a new credential": { setup: func(t *testing.T) { credentials.Store(testClusterName, &clientauthentication.ExecCredential{ TypeMeta: metav1.TypeMeta{ @@ -103,7 +105,7 @@ func TestIssueClusterCredential(t *testing.T) { Kind: "ExecCredential", }, Status: &clientauthentication.ExecCredentialStatus{ - ExpirationTimestamp: buildMetav1Time(t, t0), + ExpirationTimestamp: &metav1.Time{Time: t0}, Token: "oldToken", }, }) @@ -121,7 +123,7 @@ func TestIssueClusterCredential(t *testing.T) { "expirationTimestamp": %q, "token": "newToken" } -}`, t0.Add(24*time.Hour).UTC().Format(time.RFC3339)), +}`, t0.Add(24*time.Hour).Format(time.RFC3339)), }, }, expected: &clientauthentication.ExecCredential{ @@ -130,7 +132,7 @@ func TestIssueClusterCredential(t *testing.T) { Kind: "ExecCredential", }, Status: &clientauthentication.ExecCredentialStatus{ - ExpirationTimestamp: buildMetav1Time(t, t0.Add(24*time.Hour)), + ExpirationTimestamp: &metav1.Time{Time: t0.Add(24 * time.Hour).Local().Truncate(time.Second)}, Token: "newToken", }, }, @@ -154,17 +156,16 @@ func TestIssueClusterCredential(t *testing.T) { assert.NoError(t, err) assert.Equal(t, tt.expected, cred) - }) - } -} - -func buildMetav1Time(t *testing.T, tm time.Time) *metav1.Time { - t.Helper() - copied, err := time.Parse(time.RFC3339, tm.Format(time.RFC3339)) - assert.NoError(t, err, "failed to parse time to RFC3339: %s", err) + if tt.expected.Status != nil && !tt.expected.Status.ExpirationTimestamp.IsZero() { + fmt.Println("Expected timestamp: ", tt.expected.Status.ExpirationTimestamp) + } - return &metav1.Time{Time: copied} + if cred.Status != nil && !cred.Status.ExpirationTimestamp.IsZero() { + fmt.Println("Got timestamp: ", cred.Status.ExpirationTimestamp) + } + }) + } } func cleanAllCache(t *testing.T) { From 25d1593d717e956c47025ca25a523be2ab37a6ca Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Wed, 28 Feb 2024 12:43:31 -0300 Subject: [PATCH 09/11] test: increasing test coverage of exec package Signed-off-by: Claudio Netto --- .../clustergateway_types_secret_test.go | 90 -------------- pkg/util/exec/exec_test.go | 117 ++++++++++++++++-- 2 files changed, 107 insertions(+), 100 deletions(-) diff --git a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go index 86c8f414..df8b268e 100644 --- a/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go +++ b/pkg/apis/cluster/v1alpha1/clustergateway_types_secret_test.go @@ -658,96 +658,6 @@ func TestBuildCredentialFromExecConfig(t *testing.T) { expectedError: "failed to decode exec config JSON from secret data: invalid character 's' looking for beginning of value", }, - { - name: "missing command property within exec config", - secret: func(s *corev1.Secret) *corev1.Secret { - s.Data["exec"] = []byte(`{}`) - return s - }, - expectedError: "missing \"command\" property on exec config object", - }, - - { - name: "failed to run external command: command not found", - secret: func(s *corev1.Secret) *corev1.Secret { - s.Data["exec"] = []byte(`{"command": "/path/to/command/not/found"}`) - return s - }, - expectedError: "exec: executable /path/to/command/not/found not found", - }, - - { - name: "failed to run external command: finished with non-zero exit code", - secret: func(s *corev1.Secret) *corev1.Secret { - s.Data["exec"] = []byte(`{"command": "false"}`) - return s - }, - expectedError: "exec: executable /usr/bin/false failed with exit code 1", - }, - - { - name: "missing API version in exec config", - secret: func(s *corev1.Secret) *corev1.Secret { - s.Data["exec"] = []byte(`{"command": "true"}`) - return s - }, - expectedError: "exec plugin: invalid apiVersion \"\"", - }, - - { - name: "invalid API version in exec config", - secret: func(s *corev1.Secret) *corev1.Secret { - s.Data["exec"] = []byte(`{"apiVersion": "example.org/v1", "command": "true"}`) - return s - }, - expectedError: "exec plugin: invalid apiVersion \"example.org/v1\"", - }, - - { - name: "invalid external command output", - secret: func(s *corev1.Secret) *corev1.Secret { - s.Data["exec"] = []byte(`{"apiVersion": "client.authentication.k8s.io/v1", "command": "echo", "args": ["invalid output"]}`) - return s - }, - expectedError: "decoding stdout: couldn't get version/kind; json parse error: json: cannot unmarshal string into Go value of type struct { APIVersion string \"json:\\\"apiVersion,omitempty\\\"\"; Kind string \"json:\\\"kind,omitempty\\\"\" }", - }, - - { - name: "API version mismatch between exec config and command output", - secret: func(s *corev1.Secret) *corev1.Secret { - s.Data["exec"] = []byte(`{"apiVersion": "client.authentication.k8s.io/v1", "command": "echo", "args": ["{\"apiVersion\": \"client.authentication.k8s.io/v1beta1\"}"]}`) - return s - }, - expectedError: "exec plugin is configured to use API version client.authentication.k8s.io/v1, plugin returned version client.authentication.k8s.io/v1beta1", - }, - - { - name: "missing status property on external command output", - secret: func(s *corev1.Secret) *corev1.Secret { - s.Data["exec"] = []byte(`{"apiVersion": "client.authentication.k8s.io/v1", "command": "echo", "args": ["{\"apiVersion\": \"client.authentication.k8s.io/v1\"}"]}`) - return s - }, - expectedError: "exec plugin didn't return a status field", - }, - - { - name: "missing any auth credential on status", - secret: func(s *corev1.Secret) *corev1.Secret { - s.Data["exec"] = []byte(`{"apiVersion": "client.authentication.k8s.io/v1", "command": "echo", "args": ["{\"apiVersion\": \"client.authentication.k8s.io/v1\", \"status\": {}}"]}`) - return s - }, - expectedError: "exec plugin didn't return a token or cert/key pair", - }, - - { - name: "has cert but no private key", - secret: func(s *corev1.Secret) *corev1.Secret { - s.Data["exec"] = []byte(`{"apiVersion": "client.authentication.k8s.io/v1", "command": "echo", "args": ["{\"apiVersion\": \"client.authentication.k8s.io/v1\", \"status\": {\"clientCertificateData\": \"certData\"}}"]}`) - return s - }, - expectedError: "exec plugin returned only certificate or key, not both", - }, - { name: "returns successfully a service account token", secret: func(s *corev1.Secret) *corev1.Secret { diff --git a/pkg/util/exec/exec_test.go b/pkg/util/exec/exec_test.go index 8823bed5..0bac304a 100644 --- a/pkg/util/exec/exec_test.go +++ b/pkg/util/exec/exec_test.go @@ -37,18 +37,123 @@ func TestIssueClusterCredential(t *testing.T) { expectedError: "exec config not provided", }, - "missing command on exec config": { + "missing command property within exec config": { clusterName: testClusterName, execConfig: &clientcmdapi.ExecConfig{}, expectedError: "missing \"command\" property on exec config object", }, - "MISS credential from cache, should issue a new credential": { + "failed to run external command: command not found": { + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + Command: "/path/to/command/not/found", + }, + expectedError: "exec: executable /path/to/command/not/found not found", + }, + + "failed to run external command: finished with non-zero exit code": { + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1", + Command: "false", + }, + expectedError: "exec: executable /usr/bin/false failed with exit code 1", + }, + + "missing API version in exec config": { + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + Command: "true", + }, + expectedError: `exec plugin: invalid apiVersion ""`, + }, + + "invalid API version in exec config": { + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + APIVersion: "example.org/v1", + Command: "true", + }, + expectedError: `exec plugin: invalid apiVersion "example.org/v1"`, + }, + + "invalid exec credential JSON": { + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1", + Command: "echo", + Args: []string{"-n", `[]`}, + }, + expectedError: "decoding stdout: couldn't get version/kind; json parse error: json: cannot unmarshal array into Go value of type struct { APIVersion string \"json:\\\"apiVersion,omitempty\\\"\"; Kind string \"json:\\\"kind,omitempty\\\"\" }", + }, + + "API version mismatch": { clusterName: testClusterName, execConfig: &clientcmdapi.ExecConfig{ APIVersion: "client.authentication.k8s.io/v1", Command: "echo", Args: []string{"-n", `{ + "apiVersion": "client.authentication.k8s.io/v1beta1", + "kind": "ExecCredential", + "status": { + "token": "testToken" + } +}`}, + }, + expectedError: "exec plugin is configured to use API version client.authentication.k8s.io/v1, plugin returned version client.authentication.k8s.io/v1beta1", + }, + + "missing status property on external command output": { + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1", + Command: "echo", + Args: []string{"-n", `{"apiVersion": "client.authentication.k8s.io/v1", "kind": "ExecCredential"}`}, + }, + expectedError: "exec plugin didn't return a status field", + }, + + "missing any auth credential on status": { + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1", + Command: "echo", + Args: []string{"-n", `{"apiVersion": "client.authentication.k8s.io/v1", "kind": "ExecCredential", "status": {}}`}, + }, + expectedError: "exec plugin didn't return a token or cert/key pair", + }, + + "has cert but no private key": { + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1", + Command: "echo", + Args: []string{"-n", `{"apiVersion": "client.authentication.k8s.io/v1", "kind": "ExecCredential", "status": {"clientCertificateData": "certData"}}`}, + }, + expectedError: "exec plugin returned only certificate or key, not both", + }, + + "invalid exec credential item on cache": { + setup: func(t *testing.T) { + credentials.Store(testClusterName, "invalid exec credential") + }, + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1", + Command: "should_be_ignored", + }, + expectedError: "failed to convert item in cache to ExecCredential", + }, + + "MISS credential from cache, should issue a new credential": { + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + APIVersion: "client.authentication.k8s.io/v1", + Env: []clientcmdapi.ExecEnvVar{ + {Name: "TOKEN", Value: "testToken"}, + }, + Command: "echo", + Args: []string{"-n", `{ "apiVersion": "client.authentication.k8s.io/v1", "kind": "ExecCredential", "status": { @@ -156,14 +261,6 @@ func TestIssueClusterCredential(t *testing.T) { assert.NoError(t, err) assert.Equal(t, tt.expected, cred) - - if tt.expected.Status != nil && !tt.expected.Status.ExpirationTimestamp.IsZero() { - fmt.Println("Expected timestamp: ", tt.expected.Status.ExpirationTimestamp) - } - - if cred.Status != nil && !cred.Status.ExpirationTimestamp.IsZero() { - fmt.Println("Got timestamp: ", cred.Status.ExpirationTimestamp) - } }) } } From 03fb7c51a6c97df72a34d7b9feaf5003e1cebf06 Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Wed, 28 Feb 2024 14:41:13 -0300 Subject: [PATCH 10/11] test: cover transport package with tests about dynamic credential Signed-off-by: Claudio Netto --- pkg/apis/cluster/v1alpha1/transport_test.go | 68 +++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/pkg/apis/cluster/v1alpha1/transport_test.go b/pkg/apis/cluster/v1alpha1/transport_test.go index bc517f2c..2c3436e7 100644 --- a/pkg/apis/cluster/v1alpha1/transport_test.go +++ b/pkg/apis/cluster/v1alpha1/transport_test.go @@ -215,6 +215,74 @@ func TestClusterRestConfigConversion(t *testing.T) { }, }, }, + { + name: "dynamic credential: service account token", + clusterGateway: &ClusterGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + }, + Spec: ClusterGatewaySpec{ + Access: ClusterAccess{ + Endpoint: &ClusterEndpoint{ + Type: ClusterEndpointTypeConst, + Const: &ClusterEndpointConst{ + Address: "https://k8s.example.com", + CABundle: testCAData, + }, + }, + Credential: &ClusterAccessCredential{ + Type: CredentialTypeDynamic, + ServiceAccountToken: testToken, + }, + }, + }, + }, + expectedCfg: &rest.Config{ + Host: "https://k8s.example.com", + Timeout: 40 * time.Second, + BearerToken: testToken, + TLSClientConfig: rest.TLSClientConfig{ + ServerName: "k8s.example.com", + CAData: testCAData, + }, + }, + }, + { + name: "dynamic credential: certificate + private key", + clusterGateway: &ClusterGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + }, + Spec: ClusterGatewaySpec{ + Access: ClusterAccess{ + Endpoint: &ClusterEndpoint{ + Type: ClusterEndpointTypeConst, + Const: &ClusterEndpointConst{ + Address: "https://k8s.example.com", + CABundle: testCAData, + }, + }, + Credential: &ClusterAccessCredential{ + Type: CredentialTypeDynamic, + X509: &X509{ + Certificate: testCertData, + PrivateKey: testKeyData, + }, + }, + }, + }, + }, + expectedCfg: &rest.Config{ + Host: "https://k8s.example.com", + Timeout: 40 * time.Second, + TLSClientConfig: rest.TLSClientConfig{ + ServerName: "k8s.example.com", + CertData: testCertData, + KeyData: testKeyData, + CAData: testCAData, + }, + }, + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { From d2f8e8fe01388fbfd8832fd10e1684283f86de6f Mon Sep 17 00:00:00 2001 From: Claudio Netto Date: Wed, 28 Feb 2024 14:48:32 -0300 Subject: [PATCH 11/11] test: covering w/ test an error-catching Signed-off-by: Claudio Netto --- pkg/util/exec/exec_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/util/exec/exec_test.go b/pkg/util/exec/exec_test.go index 0bac304a..cdbe913a 100644 --- a/pkg/util/exec/exec_test.go +++ b/pkg/util/exec/exec_test.go @@ -87,6 +87,15 @@ func TestIssueClusterCredential(t *testing.T) { expectedError: "decoding stdout: couldn't get version/kind; json parse error: json: cannot unmarshal array into Go value of type struct { APIVersion string \"json:\\\"apiVersion,omitempty\\\"\"; Kind string \"json:\\\"kind,omitempty\\\"\" }", }, + "cannot parse de API version": { + clusterName: testClusterName, + execConfig: &clientcmdapi.ExecConfig{ + APIVersion: "a/b/c/d/e", + Command: "true", + }, + expectedError: "failed to parse exec config API version: unexpected GroupVersion string: a/b/c/d/e", + }, + "API version mismatch": { clusterName: testClusterName, execConfig: &clientcmdapi.ExecConfig{