From 3c3dbb47f9a8a98e3d29f31d14c7cb97ad4096b0 Mon Sep 17 00:00:00 2001 From: KevFan Date: Wed, 18 Sep 2024 10:27:10 +0100 Subject: [PATCH] feat: configurable API Key K8s secret key name Signed-off-by: KevFan --- api/v1beta1/zz_generated.deepcopy.go | 4 +- api/v1beta2/auth_config_types.go | 6 +++ api/v1beta2/zz_generated.deepcopy.go | 9 +++- controllers/auth_config_controller.go | 2 +- controllers/secret_controller_test.go | 2 +- .../authorino.kuadrant.io_authconfigs.yaml | 8 ++++ install/manifests.yaml | 8 ++++ pkg/evaluators/identity/api_key.go | 37 +++++++++------- pkg/evaluators/identity/api_key_test.go | 44 +++++++++++++++---- 9 files changed, 90 insertions(+), 30 deletions(-) diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index c70bb9df..3fe74ccf 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated /* Copyright 2020 Red Hat, Inc. @@ -123,7 +122,8 @@ func (in *AuthConfigSpec) DeepCopyInto(out *AuthConfigSpec) { if val == nil { (*out)[key] = nil } else { - in, out := &val, &outVal + inVal := (*in)[key] + in, out := &inVal, &outVal *out = make(JSONPatternExpressions, len(*in)) copy(*out, *in) } diff --git a/api/v1beta2/auth_config_types.go b/api/v1beta2/auth_config_types.go index 098ceb05..1f3d6120 100644 --- a/api/v1beta2/auth_config_types.go +++ b/api/v1beta2/auth_config_types.go @@ -343,6 +343,12 @@ type ApiKeyAuthenticationSpec struct { // +optional // +kubebuilder:default:=false AllNamespaces bool `json:"allNamespaces,omitempty"` + + // List of keys within the selected Kubernetes secret that contain valid API credentials. + // Authorino will attempt to authenticate using the first key that matches. + // If no match is found, authentication will fail. + // +optional + KeySelectors []string `json:"keySelectors,omitempty"` } // Settings to fetch the JSON Web Key Set (JWKS) for the JWT authentication. diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 3647917e..499b709d 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated /* Copyright 2020 Red Hat, Inc. @@ -50,6 +49,11 @@ func (in *ApiKeyAuthenticationSpec) DeepCopyInto(out *ApiKeyAuthenticationSpec) *out = new(v1.LabelSelector) (*in).DeepCopyInto(*out) } + if in.KeySelectors != nil { + in, out := &in.KeySelectors, &out.KeySelectors + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ApiKeyAuthenticationSpec. @@ -137,7 +141,8 @@ func (in *AuthConfigSpec) DeepCopyInto(out *AuthConfigSpec) { if val == nil { (*out)[key] = nil } else { - in, out := &val, &outVal + inVal := (*in)[key] + in, out := &inVal, &outVal *out = make(PatternExpressions, len(*in)) copy(*out, *in) } diff --git a/controllers/auth_config_controller.go b/controllers/auth_config_controller.go index cca5ff1f..067fb2c3 100644 --- a/controllers/auth_config_controller.go +++ b/controllers/auth_config_controller.go @@ -249,7 +249,7 @@ func (r *AuthConfigReconciler) translateAuthConfig(ctx context.Context, authConf if err != nil { return nil, err } - translatedIdentity.APIKey = identity_evaluators.NewApiKeyIdentity(identityCfgName, selector, namespace, authCred, r.Client, ctxWithLogger) + translatedIdentity.APIKey = identity_evaluators.NewApiKeyIdentity(identityCfgName, selector, namespace, identity.ApiKey.KeySelectors, authCred, r.Client, ctxWithLogger) // MTLS case api.X509ClientCertificateAuthentication: diff --git a/controllers/secret_controller_test.go b/controllers/secret_controller_test.go index c14d2db6..05351186 100644 --- a/controllers/secret_controller_test.go +++ b/controllers/secret_controller_test.go @@ -94,7 +94,7 @@ func newSecretReconcilerTest(mockCtrl *gomock.Controller, secretLabels map[strin indexedAuthConfig := &evaluators.AuthConfig{ Labels: map[string]string{"namespace": "authorino", "name": "api-protection"}, IdentityConfigs: []auth.AuthConfigEvaluator{&fakeAPIKeyIdentityConfig{ - evaluator: identity_evaluators.NewApiKeyIdentity("api-key", apiKeyLabelSelectors, "", auth.NewAuthCredential("", ""), fakeK8sClient, context.TODO()), + evaluator: identity_evaluators.NewApiKeyIdentity("api-key", apiKeyLabelSelectors, "", []string{}, auth.NewAuthCredential("", ""), fakeK8sClient, context.TODO()), }}, } indexMock := mock_index.NewMockIndex(mockCtrl) diff --git a/install/crd/authorino.kuadrant.io_authconfigs.yaml b/install/crd/authorino.kuadrant.io_authconfigs.yaml index 84a61661..6c09332d 100644 --- a/install/crd/authorino.kuadrant.io_authconfigs.yaml +++ b/install/crd/authorino.kuadrant.io_authconfigs.yaml @@ -2261,6 +2261,14 @@ spec: Whether Authorino should look for API key secrets in all namespaces or only in the same namespace as the AuthConfig. Enabling this option in namespaced Authorino instances has no effect. type: boolean + keySelectors: + description: |- + List of keys within the selected Kubernetes secret that contain valid API credentials. + Authorino will attempt to authenticate using the first key that matches. + If no match is found, authentication will fail. + items: + type: string + type: array selector: description: Label selector used by Authorino to match secrets from the cluster storing valid credentials to authenticate diff --git a/install/manifests.yaml b/install/manifests.yaml index 6c13cf5e..6ea64ad6 100644 --- a/install/manifests.yaml +++ b/install/manifests.yaml @@ -2541,6 +2541,14 @@ spec: Whether Authorino should look for API key secrets in all namespaces or only in the same namespace as the AuthConfig. Enabling this option in namespaced Authorino instances has no effect. type: boolean + keySelectors: + description: |- + List of keys within the selected Kubernetes secret that contain valid API credentials. + Authorino will attempt to authenticate using the first key that matches. + If no match is found, authentication will fail. + items: + type: string + type: array selector: description: Label selector used by Authorino to match secrets from the cluster storing valid credentials to authenticate diff --git a/pkg/evaluators/identity/api_key.go b/pkg/evaluators/identity/api_key.go index ab2a0236..885a41e8 100644 --- a/pkg/evaluators/identity/api_key.go +++ b/pkg/evaluators/identity/api_key.go @@ -26,18 +26,20 @@ type APIKey struct { Name string `yaml:"name"` LabelSelectors k8s_labels.Selector `yaml:"labelSelectors"` Namespace string `yaml:"namespace"` + KeySelectors []string `yaml:"keySelectors"` secrets map[string]k8s.Secret mutex sync.RWMutex k8sClient k8s_client.Reader } -func NewApiKeyIdentity(name string, labelSelectors k8s_labels.Selector, namespace string, authCred auth.AuthCredentials, k8sClient k8s_client.Reader, ctx context.Context) *APIKey { +func NewApiKeyIdentity(name string, labelSelectors k8s_labels.Selector, namespace string, keySelectors []string, authCred auth.AuthCredentials, k8sClient k8s_client.Reader, ctx context.Context) *APIKey { apiKey := &APIKey{ AuthCredentials: authCred, Name: name, LabelSelectors: labelSelectors, Namespace: namespace, + KeySelectors: append(keySelectors, apiKeySelector), secrets: make(map[string]k8s.Secret), k8sClient: k8sClient, } @@ -103,17 +105,19 @@ func (a *APIKey) AddK8sSecretBasedIdentity(ctx context.Context, new k8s.Secret) logger := log.FromContext(ctx).WithName("apikey") // updating existing - newAPIKeyValue := string(new.Data[apiKeySelector]) - for oldAPIKeyValue, current := range a.secrets { - if current.GetNamespace() == new.GetNamespace() && current.GetName() == new.GetName() { - if oldAPIKeyValue != newAPIKeyValue { - a.appendK8sSecretBasedIdentity(new) - delete(a.secrets, oldAPIKeyValue) - logger.V(1).Info("api key updated") - } else { - logger.V(1).Info("api key unchanged") + for _, key := range a.KeySelectors { + newAPIKeyValue := string(new.Data[key]) + for oldAPIKeyValue, current := range a.secrets { + if current.GetNamespace() == new.GetNamespace() && current.GetName() == new.GetName() { + if oldAPIKeyValue != newAPIKeyValue { + a.appendK8sSecretBasedIdentity(new) + delete(a.secrets, oldAPIKeyValue) + logger.V(1).Info("api key updated") + } else { + logger.V(1).Info("api key unchanged") + } + return } - return } } @@ -146,10 +150,13 @@ func (a *APIKey) withinScope(namespace string) bool { // Appends the K8s Secret to the cache of API keys // Caution! This function is not thread-safe. Make sure to acquire a lock before calling it. func (a *APIKey) appendK8sSecretBasedIdentity(secret k8s.Secret) bool { - value, isAPIKeySecret := secret.Data[apiKeySelector] - if isAPIKeySecret && len(value) > 0 { - a.secrets[string(value)] = secret - return true + for _, key := range a.KeySelectors { + value, isAPIKeySecret := secret.Data[key] + if isAPIKeySecret && len(value) > 0 { + a.secrets[string(value)] = secret + return true + } } + return false } diff --git a/pkg/evaluators/identity/api_key_test.go b/pkg/evaluators/identity/api_key_test.go index 8c569280..2d50f50b 100644 --- a/pkg/evaluators/identity/api_key_test.go +++ b/pkg/evaluators/identity/api_key_test.go @@ -11,7 +11,7 @@ import ( k8s_meta "k8s.io/apimachinery/pkg/apis/meta/v1" k8s_labels "k8s.io/apimachinery/pkg/labels" - gomock "github.com/golang/mock/gomock" + "github.com/golang/mock/gomock" "gotest.tools/assert" ) @@ -32,11 +32,13 @@ func TestNewApiKeyIdentityAllNamespaces(t *testing.T) { defer ctrl.Finish() selector, _ := k8s_labels.Parse("planet=coruscant") - apiKey := NewApiKeyIdentity("jedi", selector, "", mock_auth.NewMockAuthCredentials(ctrl), testAPIKeyK8sClient, context.TODO()) + apiKey := NewApiKeyIdentity("jedi", selector, "", []string{}, mock_auth.NewMockAuthCredentials(ctrl), testAPIKeyK8sClient, context.TODO()) assert.Equal(t, apiKey.Name, "jedi") assert.Equal(t, apiKey.LabelSelectors.String(), "planet=coruscant") assert.Equal(t, apiKey.Namespace, "") + assert.Equal(t, len(apiKey.KeySelectors), 1) + assert.Equal(t, apiKey.KeySelectors[0], apiKeySelector) assert.Equal(t, len(apiKey.secrets), 2) _, exists := apiKey.secrets["ObiWanKenobiLightSaber"] assert.Check(t, exists) @@ -51,11 +53,35 @@ func TestNewApiKeyIdentitySingleNamespace(t *testing.T) { defer ctrl.Finish() selector, _ := k8s_labels.Parse("planet=coruscant") - apiKey := NewApiKeyIdentity("jedi", selector, "ns1", mock_auth.NewMockAuthCredentials(ctrl), testAPIKeyK8sClient, context.TODO()) + apiKey := NewApiKeyIdentity("jedi", selector, "ns1", []string{}, mock_auth.NewMockAuthCredentials(ctrl), testAPIKeyK8sClient, context.TODO()) assert.Equal(t, apiKey.Name, "jedi") assert.Equal(t, apiKey.LabelSelectors.String(), "planet=coruscant") assert.Equal(t, apiKey.Namespace, "ns1") + assert.Equal(t, len(apiKey.KeySelectors), 1) + assert.Equal(t, apiKey.KeySelectors[0], apiKeySelector) + assert.Equal(t, len(apiKey.secrets), 1) + _, exists := apiKey.secrets["ObiWanKenobiLightSaber"] + assert.Check(t, exists) + _, exists = apiKey.secrets["MasterYodaLightSaber"] + assert.Check(t, !exists) + _, exists = apiKey.secrets["AnakinSkywalkerLightSaber"] + assert.Check(t, !exists) +} + +func TestNewApiKeyIdentityMultipleKeySelectors(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + selector, _ := k8s_labels.Parse("planet=coruscant") + apiKey := NewApiKeyIdentity("jedi", selector, "ns1", []string{"no_op"}, mock_auth.NewMockAuthCredentials(ctrl), testAPIKeyK8sClient, context.TODO()) + + assert.Equal(t, apiKey.Name, "jedi") + assert.Equal(t, apiKey.LabelSelectors.String(), "planet=coruscant") + assert.Equal(t, apiKey.Namespace, "ns1") + assert.Equal(t, len(apiKey.KeySelectors), 2) + assert.Equal(t, apiKey.KeySelectors[0], "no_op") + assert.Equal(t, apiKey.KeySelectors[1], apiKeySelector) assert.Equal(t, len(apiKey.secrets), 1) _, exists := apiKey.secrets["ObiWanKenobiLightSaber"] assert.Check(t, exists) @@ -74,7 +100,7 @@ func TestCallSuccess(t *testing.T) { authCredMock.EXPECT().GetCredentialsFromReq(gomock.Any()).Return("ObiWanKenobiLightSaber", nil) selector, _ := k8s_labels.Parse("planet=coruscant") - apiKey := NewApiKeyIdentity("jedi", selector, "", authCredMock, testAPIKeyK8sClient, context.TODO()) + apiKey := NewApiKeyIdentity("jedi", selector, "", []string{}, authCredMock, testAPIKeyK8sClient, context.TODO()) auth, err := apiKey.Call(pipelineMock, context.TODO()) assert.NilError(t, err) @@ -90,7 +116,7 @@ func TestCallNoApiKeyFail(t *testing.T) { authCredMock.EXPECT().GetCredentialsFromReq(gomock.Any()).Return("", fmt.Errorf("something went wrong getting the API Key")) selector, _ := k8s_labels.Parse("planet=coruscant") - apiKey := NewApiKeyIdentity("jedi", selector, "", authCredMock, testAPIKeyK8sClient, context.TODO()) + apiKey := NewApiKeyIdentity("jedi", selector, "", []string{}, authCredMock, testAPIKeyK8sClient, context.TODO()) _, err := apiKey.Call(pipelineMock, context.TODO()) @@ -106,7 +132,7 @@ func TestCallInvalidApiKeyFail(t *testing.T) { authCredMock.EXPECT().GetCredentialsFromReq(gomock.Any()).Return("ASithLightSaber", nil) selector, _ := k8s_labels.Parse("planet=coruscant") - apiKey := NewApiKeyIdentity("jedi", selector, "", authCredMock, testAPIKeyK8sClient, context.TODO()) + apiKey := NewApiKeyIdentity("jedi", selector, "", []string{}, authCredMock, testAPIKeyK8sClient, context.TODO()) _, err := apiKey.Call(pipelineMock, context.TODO()) assert.Error(t, err, "the API Key provided is invalid") @@ -114,7 +140,7 @@ func TestCallInvalidApiKeyFail(t *testing.T) { func TestLoadSecretsSuccess(t *testing.T) { selector, _ := k8s_labels.Parse("planet=coruscant") - apiKey := NewApiKeyIdentity("X-API-KEY", selector, "", nil, testAPIKeyK8sClient, nil) + apiKey := NewApiKeyIdentity("X-API-KEY", selector, "", []string{}, nil, testAPIKeyK8sClient, nil) err := apiKey.loadSecrets(context.TODO()) assert.NilError(t, err) @@ -131,7 +157,7 @@ func TestLoadSecretsSuccess(t *testing.T) { func TestLoadSecretsFail(t *testing.T) { selector, _ := k8s_labels.Parse("planet=coruscant") - apiKey := NewApiKeyIdentity("X-API-KEY", selector, "", nil, &flawedAPIkeyK8sClient{}, context.TODO()) + apiKey := NewApiKeyIdentity("X-API-KEY", selector, "", []string{}, nil, &flawedAPIkeyK8sClient{}, context.TODO()) err := apiKey.loadSecrets(context.TODO()) assert.Error(t, err, "something terribly wrong happened") @@ -145,7 +171,7 @@ func BenchmarkAPIKeyAuthn(b *testing.B) { authCredMock := mock_auth.NewMockAuthCredentials(ctrl) authCredMock.EXPECT().GetCredentialsFromReq(gomock.Any()).Return("ObiWanKenobiLightSaber", nil).MinTimes(1) selector, _ := k8s_labels.Parse("planet=coruscant") - apiKey := NewApiKeyIdentity("jedi", selector, "", authCredMock, testAPIKeyK8sClient, context.TODO()) + apiKey := NewApiKeyIdentity("jedi", selector, "", []string{}, authCredMock, testAPIKeyK8sClient, context.TODO()) var err error b.ResetTimer()