Skip to content

Commit

Permalink
refactor: CEL expression for keySelector
Browse files Browse the repository at this point in the history
Signed-off-by: KevFan <[email protected]>
  • Loading branch information
KevFan committed Feb 7, 2025
1 parent c6650df commit cc84bee
Show file tree
Hide file tree
Showing 9 changed files with 160 additions and 74 deletions.
7 changes: 5 additions & 2 deletions api/v1beta3/auth_config_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,11 +356,14 @@ type ApiKeyAuthenticationSpec struct {
// +kubebuilder:default:=false
AllNamespaces bool `json:"allNamespaces,omitempty"`

// List of keys within the selected Kubernetes secret that contain valid API credentials.
// A Common Expression Language (CEL) expression that evaluates to a list of string keys within the selected Kubernetes
// secret that contain valid API credentials. The keys of the selected Kubernetes secret are available for evaluation
// in the following structure: `{"keys": ["key1", "key2"]}`.
// Authorino will attempt to authenticate using any matching key. If no keys are defined, the default "api-key" will be used.
// If no match is found, the Kubernetes secret is not considered a valid Authorino API Key secret and is ignored.
// String expressions are supported (https://pkg.go.dev/github.com/google/cel-go/ext#Strings).
// +optional
KeySelectors []string `json:"keySelectors,omitempty"`
KeySelector CelExpression `json:"keySelector,omitempty"`
}

// Settings to fetch the JSON Web Key Set (JWKS) for the JWT authentication.
Expand Down
5 changes: 0 additions & 5 deletions api/v1beta3/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion controllers/auth_config_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,8 @@ func (r *AuthConfigReconciler) translateAuthConfig(ctx context.Context, authConf
if err != nil {
return nil, err
}
translatedIdentity.APIKey = identity_evaluators.NewApiKeyIdentity(identityCfgName, selector, namespace, identity.ApiKey.KeySelectors, authCred, r.Client, ctxWithLogger)

translatedIdentity.APIKey = identity_evaluators.NewApiKeyIdentity(identityCfgName, selector, namespace, string(identity.ApiKey.KeySelector), authCred, r.Client, ctxWithLogger)

// MTLS
case api.X509ClientCertificateAuthentication:
Expand Down
2 changes: 1 addition & 1 deletion controllers/secret_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, "", []string{}, auth.NewAuthCredential("", ""), fakeK8sClient, context.TODO()),
evaluator: identity_evaluators.NewApiKeyIdentity("api-key", apiKeyLabelSelectors, "", "", auth.NewAuthCredential("", ""), fakeK8sClient, context.TODO()),
}},
}
indexMock := mock_index.NewMockIndex(mockCtrl)
Expand Down
11 changes: 6 additions & 5 deletions install/crd/authorino.kuadrant.io_authconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2393,14 +2393,15 @@ 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:
keySelector:
description: |-
List of keys within the selected Kubernetes secret that contain valid API credentials.
A Common Expression Language (CEL) expression that evaluates to a list of string keys within the selected Kubernetes
secret that contain valid API credentials. The keys of the selected Kubernetes secret are available for evaluation
in the following structure: `{"keys": ["key1", "key2"]}`.
Authorino will attempt to authenticate using any matching key. If no keys are defined, the default "api-key" will be used.
If no match is found, the Kubernetes secret is not considered a valid Authorino API Key secret and is ignored.
items:
type: string
type: array
String expressions are supported (https://pkg.go.dev/github.com/google/cel-go/ext#Strings).
type: string
selector:
description: Label selector used by Authorino to match secrets
from the cluster storing valid credentials to authenticate
Expand Down
11 changes: 6 additions & 5 deletions install/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2660,14 +2660,15 @@ 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:
keySelector:
description: |-
List of keys within the selected Kubernetes secret that contain valid API credentials.
A Common Expression Language (CEL) expression that evaluates to a list of string keys within the selected Kubernetes
secret that contain valid API credentials. The keys of the selected Kubernetes secret are available for evaluation
in the following structure: `{"keys": ["key1", "key2"]}`.
Authorino will attempt to authenticate using any matching key. If no keys are defined, the default "api-key" will be used.
If no match is found, the Kubernetes secret is not considered a valid Authorino API Key secret and is ignored.
items:
type: string
type: array
String expressions are supported (https://pkg.go.dev/github.com/google/cel-go/ext#Strings).
type: string
selector:
description: Label selector used by Authorino to match secrets
from the cluster storing valid credentials to authenticate
Expand Down
125 changes: 92 additions & 33 deletions pkg/evaluators/identity/api_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,17 @@ package identity

import (
"context"
"encoding/json"
"fmt"
"sync"

"github.com/google/cel-go/common/types"
"github.com/google/cel-go/common/types/ref"
"github.com/samber/lo"

"github.com/kuadrant/authorino/pkg/auth"
"github.com/kuadrant/authorino/pkg/expressions"
"github.com/kuadrant/authorino/pkg/expressions/cel"
"github.com/kuadrant/authorino/pkg/log"

k8s "k8s.io/api/core/v1"
Expand All @@ -17,40 +22,49 @@ import (
)

const (
defaultAPIKeySelector = "api_key"
invalidApiKeyMsg = "the API Key provided is invalid"
credentialsFetchingErrorMsg = "Something went wrong fetching the authorized credentials"
defaultKeySelectorExpression = `['api_key']`
invalidApiKeyMsg = "the API Key provided is invalid"
credentialsFetchingErrorMsg = "Something went wrong fetching the authorized credentials"
)

type APIKey struct {
auth.AuthCredentials

Name string `yaml:"name"`
LabelSelectors k8s_labels.Selector `yaml:"labelSelectors"`
Namespace string `yaml:"namespace"`
KeySelectors []string `yaml:"keySelectors"`
Name string `yaml:"name"`
LabelSelectors k8s_labels.Selector `yaml:"labelSelectors"`
Namespace string `yaml:"namespace"`
KeySelectorExpression expressions.Value `yaml:"keySelector"`

// Map of API Key value to secret
secrets map[string]k8s.Secret
mutex sync.RWMutex
k8sClient k8s_client.Reader
}

func NewApiKeyIdentity(name string, labelSelectors k8s_labels.Selector, namespace string, keySelectors []string, authCred auth.AuthCredentials, k8sClient k8s_client.Reader, ctx context.Context) *APIKey {
if len(keySelectors) == 0 {
keySelectors = append(keySelectors, defaultAPIKeySelector)
func NewApiKeyIdentity(name string, labelSelectors k8s_labels.Selector, namespace string, keySelectorExpression string, authCred auth.AuthCredentials, k8sClient k8s_client.Reader, ctx context.Context) *APIKey {
if keySelectorExpression == "" {
keySelectorExpression = defaultKeySelectorExpression
}

logger := log.FromContext(ctx).WithName("apikey")

expr, err := cel.NewKeySelectorExpression(keySelectorExpression)
if err != nil {
logger.Error(err, "failed to create key selector expression")
return nil
}

apiKey := &APIKey{
AuthCredentials: authCred,
Name: name,
LabelSelectors: labelSelectors,
Namespace: namespace,
KeySelectors: keySelectors,
secrets: make(map[string]k8s.Secret),
k8sClient: k8sClient,
AuthCredentials: authCred,
Name: name,
LabelSelectors: labelSelectors,
Namespace: namespace,
KeySelectorExpression: expr,
secrets: make(map[string]k8s.Secret),
k8sClient: k8sClient,
}
if err := apiKey.loadSecrets(context.TODO()); err != nil {
log.FromContext(ctx).WithName("apikey").Error(err, credentialsFetchingErrorMsg)
logger.Error(err, credentialsFetchingErrorMsg)
}
return apiKey
}
Expand All @@ -70,7 +84,7 @@ func (a *APIKey) loadSecrets(ctx context.Context) error {
defer a.mutex.Unlock()

for _, secret := range secretList.Items {
a.appendK8sSecretBasedIdentity(secret)
a.appendK8sSecretBasedIdentity(ctx, secret)
}

return nil
Expand Down Expand Up @@ -105,9 +119,6 @@ func (a *APIKey) AddK8sSecretBasedIdentity(ctx context.Context, new k8s.Secret)
return
}

a.mutex.Lock()
defer a.mutex.Unlock()

logger := log.FromContext(ctx).WithName("apikey")

// Get all current keys in the map that match the new secret name and namespace
Expand All @@ -116,7 +127,10 @@ func (a *APIKey) AddK8sSecretBasedIdentity(ctx context.Context, new k8s.Secret)
})

// get api keys from new secret
newAPIKeys := a.getValuesFromSecret(new)
newAPIKeys := a.getValuesFromSecret(ctx, new)

a.mutex.Lock()
defer a.mutex.Unlock()

for _, newKey := range newAPIKeys {
a.secrets[newKey] = new
Expand Down Expand Up @@ -157,8 +171,8 @@ 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 {
values := a.getValuesFromSecret(secret)
func (a *APIKey) appendK8sSecretBasedIdentity(ctx context.Context, secret k8s.Secret) bool {
values := a.getValuesFromSecret(ctx, secret)
for _, value := range values {
a.secrets[value] = secret
}
Expand All @@ -167,16 +181,61 @@ func (a *APIKey) appendK8sSecretBasedIdentity(secret k8s.Secret) bool {
return len(values) != 0
}

// getValuesFromSecret extracts the values from the secret based on APIKey KeySelectors
func (a *APIKey) getValuesFromSecret(secret k8s.Secret) []string {
var keys []string
for _, key := range a.KeySelectors {
value, isAPIKeySecret := secret.Data[key]
// getValuesFromSecret extracts the values from the secret based on APIKey KeySelector expression
func (a *APIKey) getValuesFromSecret(ctx context.Context, secret k8s.Secret) []string {
logger := log.FromContext(ctx).WithName("apikey")

// Extract secret keys
secretKeys := lo.Keys(secret.Data)

if isAPIKeySecret && len(value) > 0 {
keys = append(keys, string(value))
// Prepare JSON for CEL evaluation
jsonBytes, err := json.Marshal(map[string][]string{cel.RootSecretKeysBinding: secretKeys})
if err != nil {
logger.Error(err, "failed to marshal secret keys to JSON")
return nil
}

// Evaluate CEL expression
evaluated, err := a.KeySelectorExpression.ResolveFor(string(jsonBytes))
if err != nil {
logger.Error(err, "failed to resolve key selector expression")
return nil
}

// Convert evaluated result to a slice of strings
selectedKeys, ok := convertToStringSlice(evaluated)
if !ok {
logger.Error(fmt.Errorf("unexpected type for resolved key"), "expected string or []string", "value", evaluated)
return nil
}

// Extract values for the selected keys
values := make([]string, 0, len(selectedKeys))
for _, key := range selectedKeys {
if v, exists := secret.Data[key]; exists && len(v) > 0 {
values = append(values, string(v))
}
}

return values
}

// Helper function to safely convert an interface{} of type []ref.Val to []string
func convertToStringSlice(value any) ([]string, bool) {
items, ok := value.([]ref.Val)
if !ok {
return nil, false
}

out := make([]string, len(items))
for i, item := range items {
if item.Type() == types.StringType {
out[i] = item.Value().(string)
} else {
// unexpected type
return nil, false
}
}

return keys
return out, true
}
27 changes: 10 additions & 17 deletions pkg/evaluators/identity/api_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ var (
)

func TestConstants(t *testing.T) {
assert.Equal(t, defaultAPIKeySelector, "api_key")
assert.Equal(t, defaultKeySelectorExpression, `['api_key']`)
assert.Equal(t, invalidApiKeyMsg, "the API Key provided is invalid")
}

Expand All @@ -32,13 +32,11 @@ func TestNewApiKeyIdentityAllNamespaces(t *testing.T) {
defer ctrl.Finish()

selector, _ := k8s_labels.Parse("planet=coruscant")
apiKey := NewApiKeyIdentity("jedi", selector, "", []string{}, mock_auth.NewMockAuthCredentials(ctrl), testAPIKeyK8sClient, context.TODO())
apiKey := NewApiKeyIdentity("jedi", selector, "", "", 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], defaultAPIKeySelector)
assert.Equal(t, len(apiKey.secrets), 2)
_, exists := apiKey.secrets["ObiWanKenobiLightSaber"]
assert.Check(t, exists)
Expand All @@ -53,13 +51,11 @@ func TestNewApiKeyIdentitySingleNamespace(t *testing.T) {
defer ctrl.Finish()

selector, _ := k8s_labels.Parse("planet=coruscant")
apiKey := NewApiKeyIdentity("jedi", selector, "ns1", []string{}, mock_auth.NewMockAuthCredentials(ctrl), testAPIKeyK8sClient, context.TODO())
apiKey := NewApiKeyIdentity("jedi", selector, "ns1", "", 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], defaultAPIKeySelector)
assert.Equal(t, len(apiKey.secrets), 1)
_, exists := apiKey.secrets["ObiWanKenobiLightSaber"]
assert.Check(t, exists)
Expand All @@ -74,14 +70,11 @@ func TestNewApiKeyIdentityMultipleKeySelectors(t *testing.T) {
defer ctrl.Finish()

selector, _ := k8s_labels.Parse("planet=coruscant")
apiKey := NewApiKeyIdentity("jedi", selector, "ns1", []string{"no_op", "api_key_2"}, mock_auth.NewMockAuthCredentials(ctrl), testAPIKeyK8sClient, context.TODO())
apiKey := NewApiKeyIdentity("jedi", selector, "ns1", "['no_op','api_key_2']", 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], "api_key_2")
assert.Equal(t, len(apiKey.secrets), 1)
_, exists := apiKey.secrets["ObiWanKenobiLightSaber"]
assert.Check(t, !exists)
Expand All @@ -102,7 +95,7 @@ func TestCallSuccess(t *testing.T) {
authCredMock.EXPECT().GetCredentialsFromReq(gomock.Any()).Return("ObiWanKenobiLightSaber", nil)

selector, _ := k8s_labels.Parse("planet=coruscant")
apiKey := NewApiKeyIdentity("jedi", selector, "", []string{}, authCredMock, testAPIKeyK8sClient, context.TODO())
apiKey := NewApiKeyIdentity("jedi", selector, "", "", authCredMock, testAPIKeyK8sClient, context.TODO())
auth, err := apiKey.Call(pipelineMock, context.TODO())

assert.NilError(t, err)
Expand All @@ -118,7 +111,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, "", []string{}, authCredMock, testAPIKeyK8sClient, context.TODO())
apiKey := NewApiKeyIdentity("jedi", selector, "", "", authCredMock, testAPIKeyK8sClient, context.TODO())

_, err := apiKey.Call(pipelineMock, context.TODO())

Expand All @@ -134,15 +127,15 @@ func TestCallInvalidApiKeyFail(t *testing.T) {
authCredMock.EXPECT().GetCredentialsFromReq(gomock.Any()).Return("ASithLightSaber", nil)

selector, _ := k8s_labels.Parse("planet=coruscant")
apiKey := NewApiKeyIdentity("jedi", selector, "", []string{}, authCredMock, testAPIKeyK8sClient, context.TODO())
apiKey := NewApiKeyIdentity("jedi", selector, "", "", authCredMock, testAPIKeyK8sClient, context.TODO())
_, err := apiKey.Call(pipelineMock, context.TODO())

assert.Error(t, err, "the API Key provided is invalid")
}

func TestLoadSecretsSuccess(t *testing.T) {
selector, _ := k8s_labels.Parse("planet=coruscant")
apiKey := NewApiKeyIdentity("X-API-KEY", selector, "", []string{}, nil, testAPIKeyK8sClient, nil)
apiKey := NewApiKeyIdentity("X-API-KEY", selector, "", "", nil, testAPIKeyK8sClient, nil)

err := apiKey.loadSecrets(context.TODO())
assert.NilError(t, err)
Expand All @@ -159,7 +152,7 @@ func TestLoadSecretsSuccess(t *testing.T) {

func TestLoadSecretsFail(t *testing.T) {
selector, _ := k8s_labels.Parse("planet=coruscant")
apiKey := NewApiKeyIdentity("X-API-KEY", selector, "", []string{}, nil, &flawedAPIkeyK8sClient{}, context.TODO())
apiKey := NewApiKeyIdentity("X-API-KEY", selector, "", "", nil, &flawedAPIkeyK8sClient{}, context.TODO())

err := apiKey.loadSecrets(context.TODO())
assert.Error(t, err, "something terribly wrong happened")
Expand All @@ -173,7 +166,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, "", []string{}, authCredMock, testAPIKeyK8sClient, context.TODO())
apiKey := NewApiKeyIdentity("jedi", selector, "", "", authCredMock, testAPIKeyK8sClient, context.TODO())

var err error
b.ResetTimer()
Expand Down
Loading

0 comments on commit cc84bee

Please sign in to comment.