From b42bafaa6dd19bcc88395fdd92e5819189e8e63f Mon Sep 17 00:00:00 2001 From: Jim Fitzpatrick Date: Thu, 30 Nov 2023 15:10:03 +0000 Subject: [PATCH] Reconcile authorino spec from kuadrant CR --- api/v1beta1/kuadrant_types.go | 13 +- api/v1beta1/zz_generated.deepcopy.go | 37 +++- config/crd/bases/kuadrant.io_kuadrants.yaml | 5 - controllers/kuadrant_controller.go | 83 +++++--- pkg/common/common.go | 1 + pkg/kuadranttools/authorino_tools.go | 70 +++++++ pkg/kuadranttools/authorino_tools_test.go | 200 ++++++++++++++++++++ 7 files changed, 377 insertions(+), 32 deletions(-) create mode 100644 pkg/kuadranttools/authorino_tools.go create mode 100644 pkg/kuadranttools/authorino_tools_test.go diff --git a/api/v1beta1/kuadrant_types.go b/api/v1beta1/kuadrant_types.go index 984dc63c5..21c23faa8 100644 --- a/api/v1beta1/kuadrant_types.go +++ b/api/v1beta1/kuadrant_types.go @@ -35,7 +35,7 @@ type KuadrantSpec struct { type AuthorinoSpec struct { EvaluatorCacheSize *int `json:"evaluatorCacheSize,omitempty"` - Listener *authorinov1beta1.Listener `json:"listener,omitempty"` + Listener *AuthorinoListener `json:"listener,omitempty"` Metrics *authorinov1beta1.Metrics `json:"metrics,omitempty"` OIDCServer *authorinov1beta1.OIDCServer `json:"oidcServer,omitempty"` Replicas *int32 `json:"replicas,omitempty"` @@ -43,6 +43,17 @@ type AuthorinoSpec struct { Volumes *authorinov1beta1.VolumesSpec `json:"volumes,omitempty"` } +type AuthorinoListener struct { + // Port numbers of the GRPC and HTTP auth interfaces. + Ports *authorinov1beta1.Ports `json:"ports,omitempty"` + // TLS configuration of the auth service (GRPC and HTTP interfaces). + Tls *authorinov1beta1.Tls `json:"tls"` + // Timeout of the auth service (GRPC and HTTP interfaces), in milliseconds. + Timeout *int `json:"timeout,omitempty"` + // Maximum payload (request body) size for the auth service (HTTP interface), in bytes. + MaxHttpRequestBodySize *int `json:"maxHttpRequestBodySize,omitempty"` +} + // KuadrantStatus defines the observed state of Kuadrant type KuadrantStatus struct { // ObservedGeneration reflects the generation of the most recently observed spec. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 9103c1e38..89604b9ec 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -26,6 +26,41 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AuthorinoListener) DeepCopyInto(out *AuthorinoListener) { + *out = *in + if in.Ports != nil { + in, out := &in.Ports, &out.Ports + *out = new(apiv1beta1.Ports) + (*in).DeepCopyInto(*out) + } + if in.Tls != nil { + in, out := &in.Tls, &out.Tls + *out = new(apiv1beta1.Tls) + (*in).DeepCopyInto(*out) + } + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(int) + **out = **in + } + if in.MaxHttpRequestBodySize != nil { + in, out := &in.MaxHttpRequestBodySize, &out.MaxHttpRequestBodySize + *out = new(int) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AuthorinoListener. +func (in *AuthorinoListener) DeepCopy() *AuthorinoListener { + if in == nil { + return nil + } + out := new(AuthorinoListener) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AuthorinoSpec) DeepCopyInto(out *AuthorinoSpec) { *out = *in @@ -36,7 +71,7 @@ func (in *AuthorinoSpec) DeepCopyInto(out *AuthorinoSpec) { } if in.Listener != nil { in, out := &in.Listener, &out.Listener - *out = new(apiv1beta1.Listener) + *out = new(AuthorinoListener) (*in).DeepCopyInto(*out) } if in.Metrics != nil { diff --git a/config/crd/bases/kuadrant.io_kuadrants.yaml b/config/crd/bases/kuadrant.io_kuadrants.yaml index 5aeee659e..e2c991787 100644 --- a/config/crd/bases/kuadrant.io_kuadrants.yaml +++ b/config/crd/bases/kuadrant.io_kuadrants.yaml @@ -53,11 +53,6 @@ spec: description: Maximum payload (request body) size for the auth service (HTTP interface), in bytes. type: integer - port: - description: 'Port number of the GRPC interface. DEPRECATED: - use ''ports.grpc'' instead.' - format: int32 - type: integer ports: description: Port numbers of the GRPC and HTTP auth interfaces. properties: diff --git a/controllers/kuadrant_controller.go b/controllers/kuadrant_controller.go index b43adcdc0..68c1d7040 100644 --- a/controllers/kuadrant_controller.go +++ b/controllers/kuadrant_controller.go @@ -19,7 +19,7 @@ package controllers import ( "context" "encoding/json" - + "github.com/kuadrant/kuadrant-operator/pkg/kuadranttools" corev1 "k8s.io/api/core/v1" "k8s.io/utils/env" @@ -493,38 +493,71 @@ func (r *KuadrantReconciler) reconcileLimitador(ctx context.Context, kObj *kuadr } func (r *KuadrantReconciler) reconcileAuthorino(ctx context.Context, kObj *kuadrantv1beta1.Kuadrant) error { - tmpFalse := false - authorino := &authorinov1beta1.Authorino{ - TypeMeta: metav1.TypeMeta{ - Kind: "Authorino", - APIVersion: "operator.authorino.kuadrant.io/v1beta1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "authorino", - Namespace: kObj.Namespace, - }, - Spec: authorinov1beta1.AuthorinoSpec{ - ClusterWide: true, - SupersedingHostSubsets: true, - Listener: authorinov1beta1.Listener{ - Tls: authorinov1beta1.Tls{ - Enabled: &tmpFalse, + authorinoKey := client.ObjectKey{Name: common.AuthorinoName, Namespace: kObj.Namespace} + authorino := &authorinov1beta1.Authorino{} + err := r.Client().Get(ctx, authorinoKey, authorino) + if err != nil { + if apierrors.IsNotFound(err) { + tmpFalse := false + authorino = &authorinov1beta1.Authorino{ + TypeMeta: metav1.TypeMeta{ + Kind: "Authorino", + APIVersion: "operator.authorino.kuadrant.io/v1beta1", }, - }, - OIDCServer: authorinov1beta1.OIDCServer{ - Tls: authorinov1beta1.Tls{ - Enabled: &tmpFalse, + ObjectMeta: metav1.ObjectMeta{ + Name: common.AuthorinoName, + Namespace: kObj.Namespace, }, - }, - }, + Spec: authorinov1beta1.AuthorinoSpec{ + ClusterWide: true, + SupersedingHostSubsets: true, + Listener: authorinov1beta1.Listener{ + Tls: authorinov1beta1.Tls{ + Enabled: &tmpFalse, + }, + }, + OIDCServer: authorinov1beta1.OIDCServer{ + Tls: authorinov1beta1.Tls{ + Enabled: &tmpFalse, + }, + }, + }, + } + } else { + return err + } + } + + if kObj.Spec.Authorino != nil { + if kObj.Spec.Authorino.EvaluatorCacheSize != nil { + authorino.Spec.EvaluatorCacheSize = kObj.Spec.Authorino.EvaluatorCacheSize + } + if kObj.Spec.Authorino.Metrics != nil { + authorino.Spec.Metrics = *kObj.Spec.Authorino.Metrics + } + if kObj.Spec.Authorino.Replicas != nil { + authorino.Spec.Replicas = kObj.Spec.Authorino.Replicas + } + if kObj.Spec.Authorino.Tracing != nil { + authorino.Spec.Tracing = *kObj.Spec.Authorino.Tracing + } + if kObj.Spec.Authorino.OIDCServer != nil { + authorino.Spec.OIDCServer = *kObj.Spec.Authorino.OIDCServer + } + if kObj.Spec.Authorino.Listener != nil { + authorino.Spec.Listener = kuadranttools.MapListenerSpec(&authorino.Spec.Listener, *kObj.Spec.Authorino.Listener) + } + if kObj.Spec.Authorino.Volumes != nil { + authorino.Spec.Volumes = *kObj.Spec.Authorino.Volumes + } } - err := r.SetOwnerReference(kObj, authorino) + err = r.SetOwnerReference(kObj, authorino) if err != nil { return err } - return r.ReconcileResource(ctx, &authorinov1beta1.Authorino{}, authorino, reconcilers.CreateOnlyMutator) + return r.ReconcileResource(ctx, &authorinov1beta1.Authorino{}, authorino, kuadranttools.AuthorinoMutator) } // SetupWithManager sets up the controller with the Manager. diff --git a/pkg/common/common.go b/pkg/common/common.go index 534a3ebb6..3b201c0a9 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -36,6 +36,7 @@ const ( KuadrantNamespaceLabel = "kuadrant.io/namespace" NamespaceSeparator = '/' LimitadorName = "limitador" + AuthorinoName = "authorino" ) type KuadrantPolicy interface { diff --git a/pkg/kuadranttools/authorino_tools.go b/pkg/kuadranttools/authorino_tools.go new file mode 100644 index 000000000..c98aedb06 --- /dev/null +++ b/pkg/kuadranttools/authorino_tools.go @@ -0,0 +1,70 @@ +package kuadranttools + +import ( + "fmt" + authorinov1beta1 "github.com/kuadrant/authorino-operator/api/v1beta1" + "github.com/kuadrant/kuadrant-operator/api/v1beta1" + "reflect" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func AuthorinoMutator(existingObj, desiredObj client.Object) (bool, error) { + update := false + existing, ok := existingObj.(*authorinov1beta1.Authorino) + if !ok { + return false, fmt.Errorf("existingObj %T is not a *authorinoauthorinov1beta1.Authorino", existingObj) + } + desired, ok := desiredObj.(*authorinov1beta1.Authorino) + if !ok { + return false, fmt.Errorf("desiredObj %T is not a *authorinoauthorinov1beta1.Authorino", desiredObj) + } + + existingSpec := authorinoSpecSubSet(existing.Spec) + desiredSpec := authorinoSpecSubSet(desired.Spec) + + if !reflect.DeepEqual(existingSpec, desiredSpec) { + update = true + existing.Spec.EvaluatorCacheSize = desiredSpec.EvaluatorCacheSize + existing.Spec.Listener = desiredSpec.Listener + existing.Spec.Metrics = desiredSpec.Metrics + existing.Spec.OIDCServer = desiredSpec.OIDCServer + existing.Spec.Replicas = desiredSpec.Replicas + existing.Spec.Tracing = desiredSpec.Tracing + existing.Spec.Volumes = desiredSpec.Volumes + } + return update, nil +} + +func authorinoSpecSubSet(spec authorinov1beta1.AuthorinoSpec) authorinov1beta1.AuthorinoSpec { + out := authorinov1beta1.AuthorinoSpec{} + + out.EvaluatorCacheSize = spec.EvaluatorCacheSize + out.Listener = spec.Listener + out.Metrics = spec.Metrics + out.OIDCServer = spec.OIDCServer + out.Replicas = spec.Replicas + out.Tracing = spec.Tracing + out.Volumes = spec.Volumes + + return out +} + +func MapListenerSpec(listener *authorinov1beta1.Listener, spec v1beta1.AuthorinoListener) authorinov1beta1.Listener { + out := authorinov1beta1.Listener{} + if listener != nil { + out = *listener + } + if spec.Ports != nil { + out.Ports = *spec.Ports + } + if spec.Tls != nil { + out.Tls = *spec.Tls + } + if spec.Timeout != nil { + out.Timeout = spec.Timeout + } + if spec.MaxHttpRequestBodySize != nil { + out.MaxHttpRequestBodySize = spec.MaxHttpRequestBodySize + } + return out +} diff --git a/pkg/kuadranttools/authorino_tools_test.go b/pkg/kuadranttools/authorino_tools_test.go new file mode 100644 index 000000000..55508d7fa --- /dev/null +++ b/pkg/kuadranttools/authorino_tools_test.go @@ -0,0 +1,200 @@ +package kuadranttools + +import ( + authorinov1beta1 "github.com/kuadrant/authorino-operator/api/v1beta1" + "github.com/kuadrant/kuadrant-operator/api/v1beta1" + "k8s.io/utils/ptr" + "reflect" + "sigs.k8s.io/controller-runtime/pkg/client" + "testing" +) + +func Test_authorinoSpecSubSet(t *testing.T) { + type args struct { + spec authorinov1beta1.AuthorinoSpec + } + tests := []struct { + name string + args args + want authorinov1beta1.AuthorinoSpec + }{ + { + name: "Empty spec passed", + args: args{spec: authorinov1beta1.AuthorinoSpec{}}, + want: authorinov1beta1.AuthorinoSpec{}, + }, + { + name: "Full spec passed", + args: args{spec: authorinov1beta1.AuthorinoSpec{ + EvaluatorCacheSize: ptr.To(9000), + Listener: authorinov1beta1.Listener{}, + Metrics: authorinov1beta1.Metrics{ + Port: ptr.To(int32(9000)), + DeepMetricsEnabled: ptr.To(true), + }, + OIDCServer: authorinov1beta1.OIDCServer{}, + Replicas: ptr.To(int32(3)), + Tracing: authorinov1beta1.Tracing{}, + Volumes: authorinov1beta1.VolumesSpec{}, + }, + }, + want: authorinov1beta1.AuthorinoSpec{ + EvaluatorCacheSize: ptr.To(9000), + Listener: authorinov1beta1.Listener{}, + Metrics: authorinov1beta1.Metrics{ + Port: ptr.To(int32(9000)), + DeepMetricsEnabled: ptr.To(true), + }, + OIDCServer: authorinov1beta1.OIDCServer{}, + Replicas: ptr.To(int32(3)), + Tracing: authorinov1beta1.Tracing{}, + Volumes: authorinov1beta1.VolumesSpec{}, + }, + }, + { + name: "Partial spec passed", + args: args{spec: authorinov1beta1.AuthorinoSpec{ + Replicas: ptr.To(int32(3)), + Metrics: authorinov1beta1.Metrics{ + Port: ptr.To(int32(9000)), + DeepMetricsEnabled: ptr.To(true), + }, + }, + }, + want: authorinov1beta1.AuthorinoSpec{ + Replicas: ptr.To(int32(3)), + Metrics: authorinov1beta1.Metrics{ + Port: ptr.To(int32(9000)), + DeepMetricsEnabled: ptr.To(true), + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := authorinoSpecSubSet(tt.args.spec); !reflect.DeepEqual(got, tt.want) { + t.Errorf("authorinoSpecSubSet() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestAuthorinoMutator(t *testing.T) { + type args struct { + existingObj client.Object + desiredObj client.Object + } + tests := []struct { + name string + args args + want bool + wantErr bool + errorContains string + }{ + { + name: "existingObj is not a authorino type", + wantErr: true, + errorContains: "existingObj", + }, + { + name: "desiredObj is not a authorino type", + args: args{ + existingObj: &authorinov1beta1.Authorino{}, + }, + wantErr: true, + errorContains: "desireObj", + }, + { + name: "No update required", + args: args{ + existingObj: &authorinov1beta1.Authorino{}, + desiredObj: &authorinov1beta1.Authorino{}, + }, + want: false, + }, + { + name: "Update required", + args: args{ + existingObj: &authorinov1beta1.Authorino{ + Spec: authorinov1beta1.AuthorinoSpec{ + Replicas: ptr.To(int32(3)), + }, + }, + desiredObj: &authorinov1beta1.Authorino{ + Spec: authorinov1beta1.AuthorinoSpec{ + Replicas: ptr.To(int32(1)), + }, + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := AuthorinoMutator(tt.args.existingObj, tt.args.desiredObj) + if (err != nil) != tt.wantErr { + t.Errorf("AuthorinoMutator() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("AuthorinoMutator() got = %v, want %v", got, tt.want) + } + }) + } +} + +func TestMapListenerSpec(t *testing.T) { + type args struct { + listener *authorinov1beta1.Listener + spec v1beta1.AuthorinoListener + } + tests := []struct { + name string + args args + want authorinov1beta1.Listener + }{ + { + name: "Authorino Listener is nil", + args: args{ + listener: nil, + }, + want: authorinov1beta1.Listener{}, + }, + { + name: "Authorino listener has deprecated port set", + args: args{ + listener: &authorinov1beta1.Listener{Port: ptr.To(int32(2))}, + spec: v1beta1.AuthorinoListener{Timeout: ptr.To(5000)}, + }, + want: authorinov1beta1.Listener{ + Port: ptr.To(int32(2)), + Timeout: ptr.To(5000), + }, + }, + { + name: "Past in spec copied to Authorino listener", + args: args{ + listener: nil, + spec: v1beta1.AuthorinoListener{ + Ports: &authorinov1beta1.Ports{}, + Tls: &authorinov1beta1.Tls{}, + Timeout: ptr.To(5000), + MaxHttpRequestBodySize: ptr.To(5000), + }, + }, + want: authorinov1beta1.Listener{ + Timeout: ptr.To(5000), + Ports: authorinov1beta1.Ports{}, + Tls: authorinov1beta1.Tls{}, + MaxHttpRequestBodySize: ptr.To(5000), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := MapListenerSpec(tt.args.listener, tt.args.spec); !reflect.DeepEqual(got, tt.want) { + t.Errorf("MapListenerSpec() = %v, want %v", got, tt.want) + } + }) + } +}