diff --git a/.github/workflows/build-images-base.yaml b/.github/workflows/build-images-base.yaml index 44c553fc2..48d1a4945 100644 --- a/.github/workflows/build-images-base.yaml +++ b/.github/workflows/build-images-base.yaml @@ -100,10 +100,10 @@ jobs: name: Build Bundle runs-on: ubuntu-latest steps: - - name: Set up Go 1.19.x + - name: Set up Go 1.20.x uses: actions/setup-go@v4 with: - go-version: 1.19.x + go-version: 1.20.x id: go - name: Check out code uses: actions/checkout@v3 @@ -147,10 +147,10 @@ jobs: needs: [build, build-bundle] runs-on: ubuntu-latest steps: - - name: Set up Go 1.19.x + - name: Set up Go 1.20.x uses: actions/setup-go@v4 with: - go-version: 1.19.x + go-version: 1.20.x id: go - name: Check out code uses: actions/checkout@v3 diff --git a/.github/workflows/code-style.yaml b/.github/workflows/code-style.yaml index 1b0db993f..5e12fc45c 100644 --- a/.github/workflows/code-style.yaml +++ b/.github/workflows/code-style.yaml @@ -40,10 +40,10 @@ jobs: importpath: golang.org/x/tools/cmd/goimports@latest steps: - - name: Set up Go 1.19.x + - name: Set up Go 1.20.x uses: actions/setup-go@v4 with: - go-version: 1.19.x + go-version: 1.20.x id: go - name: Check out code @@ -90,10 +90,10 @@ jobs: runs-on: ubuntu-latest steps: - - name: Set up Go 1.19.x + - name: Set up Go 1.20.x uses: actions/setup-go@v4 with: - go-version: 1.19.x + go-version: 1.20.x id: go - name: Check out code diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 349f36d22..26e814e2e 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -18,7 +18,7 @@ jobs: name: Unit Tests strategy: matrix: - go-version: [ 1.19.x ] + go-version: [ 1.20.x ] platform: [ ubuntu-latest ] runs-on: ${{ matrix.platform }} defaults: @@ -53,10 +53,10 @@ jobs: run: shell: bash steps: - - name: Set up Go 1.19.x + - name: Set up Go 1.20.x uses: actions/setup-go@v4 with: - go-version: 1.19.x + go-version: 1.20.x id: go - name: Check out code uses: actions/checkout@v3 @@ -94,10 +94,10 @@ jobs: name: Verify manifests runs-on: ubuntu-latest steps: - - name: Set up Go 1.19.x + - name: Set up Go 1.20.x uses: actions/setup-go@v4 with: - go-version: 1.19.x + go-version: 1.20.x id: go - name: Check out code uses: actions/checkout@v3 @@ -109,10 +109,10 @@ jobs: name: Verify bundle runs-on: ubuntu-latest steps: - - name: Set up Go 1.19.x + - name: Set up Go 1.20.x uses: actions/setup-go@v4 with: - go-version: 1.19.x + go-version: 1.20.x id: go - name: Check out code uses: actions/checkout@v3 @@ -124,10 +124,10 @@ jobs: name: Verify fmt runs-on: ubuntu-latest steps: - - name: Set up Go 1.19.x + - name: Set up Go 1.20.x uses: actions/setup-go@v4 with: - go-version: 1.19.x + go-version: 1.20.x id: go - name: Check out code uses: actions/checkout@v3 @@ -139,7 +139,7 @@ jobs: name: Test Scripts strategy: matrix: - go-version: [ 1.19.x ] + go-version: [ 1.20.x ] platform: [ ubuntu-latest, macos-latest ] runs-on: ${{ matrix.platform }} defaults: diff --git a/Dockerfile b/Dockerfile index 66cb82a29..7782a3b87 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build the manager binary -FROM golang:1.19 as builder +FROM golang:1.20 as builder WORKDIR /workspace # Copy the Go Modules manifests diff --git a/Makefile b/Makefile index 790b10255..4cbff0048 100644 --- a/Makefile +++ b/Makefile @@ -451,7 +451,7 @@ run-lint: $(GOLANGCI-LINT) ## Run lint tests $(GOLANGCI-LINT) run $(GOLANGCI-LINT): - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(PROJECT_PATH)/bin v1.50.1 + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(PROJECT_PATH)/bin v1.54.2 .PHONY: golangci-lint golangci-lint: $(GOLANGCI-LINT) ## Download golangci-lint locally if necessary. diff --git a/api/external/maistra/v1/helmvalues.go b/api/external/maistra/v1/helmvalues.go index 7f58c1f53..3d9007f32 100644 --- a/api/external/maistra/v1/helmvalues.go +++ b/api/external/maistra/v1/helmvalues.go @@ -13,24 +13,24 @@ import ( // +kubebuilder:validation:Type=object // +kubebuilder:validation:XPreserveUnknownFields type HelmValues struct { - data map[string]interface{} `json:"-"` + data map[string]any `json:"-"` } -func NewHelmValues(values map[string]interface{}) *HelmValues { +func NewHelmValues(values map[string]any) *HelmValues { if values == nil { - values = make(map[string]interface{}) + values = make(map[string]any) } return &HelmValues{data: values} } -func (h *HelmValues) GetContent() map[string]interface{} { +func (h *HelmValues) GetContent() map[string]any { if h == nil { return nil } return h.data } -func (h *HelmValues) GetFieldNoCopy(path string) (interface{}, bool, error) { +func (h *HelmValues) GetFieldNoCopy(path string) (any, bool, error) { if h == nil || h.data == nil { return nil, false, nil } @@ -193,7 +193,7 @@ func (h *HelmValues) GetAndRemoveStringSlice(path string) ([]string, bool, error return value, ok, err } -func (h *HelmValues) GetSlice(path string) ([]interface{}, bool, error) { +func (h *HelmValues) GetSlice(path string) ([]any, bool, error) { if h == nil || h.data == nil { return nil, false, nil } @@ -206,7 +206,7 @@ func (h *HelmValues) GetSlice(path string) ([]interface{}, bool, error) { return slice, ok, err } -func (h *HelmValues) GetAndRemoveSlice(path string) ([]interface{}, bool, error) { +func (h *HelmValues) GetAndRemoveSlice(path string) ([]any, bool, error) { value, ok, err := h.GetSlice(path) if err == nil { h.RemoveField(path) @@ -245,7 +245,7 @@ func (h *HelmValues) GetAndRemoveStringToStringMap(path string) (map[string]stri return stringToStringMap, found, nil } -func (h *HelmValues) GetMap(path string) (map[string]interface{}, bool, error) { +func (h *HelmValues) GetMap(path string) (map[string]any, bool, error) { if h == nil || h.data == nil { return nil, false, nil } @@ -254,15 +254,15 @@ func (h *HelmValues) GetMap(path string) (map[string]interface{}, bool, error) { if rawval == nil { return nil, ok, err } - if mapval, ok := rawval.(map[string]interface{}); ok { + if mapval, ok := rawval.(map[string]any); ok { return mapval, ok, err } - return nil, false, fmt.Errorf("%v accessor error: %v is of the type %T, expected map[string]interface{}", path, rawval, rawval) + return nil, false, fmt.Errorf("%v accessor error: %v is of the type %T, expected map[string]any", path, rawval, rawval) } return nil, ok, err } -func (h *HelmValues) GetAndRemoveMap(path string) (map[string]interface{}, bool, error) { +func (h *HelmValues) GetAndRemoveMap(path string) (map[string]any, bool, error) { value, ok, err := h.GetMap(path) if err == nil { h.RemoveField(path) @@ -289,12 +289,12 @@ func (h *HelmValues) GetAndRemoveStringMap(path string) (map[string]string, bool return value, ok, err } -func (h *HelmValues) SetField(path string, value interface{}) error { +func (h *HelmValues) SetField(path string, value any) error { if h == nil { panic("Tried to invoke SetField on nil *HelmValues") } if h.data == nil { - h.data = map[string]interface{}{} + h.data = map[string]any{} } return unstructured.SetNestedField(h.data, value, strings.Split(path, ".")...) } @@ -304,7 +304,7 @@ func (h *HelmValues) SetStringSlice(path string, value []string) error { panic("Tried to invoke SetField on nil *HelmValues") } if h.data == nil { - h.data = map[string]interface{}{} + h.data = map[string]any{} } return unstructured.SetNestedStringSlice(h.data, value, strings.Split(path, ".")...) } diff --git a/controllers/authpolicy_auth_config.go b/controllers/authpolicy_auth_config.go index 675769705..56476d6ed 100644 --- a/controllers/authpolicy_auth_config.go +++ b/controllers/authpolicy_auth_config.go @@ -94,7 +94,7 @@ func (r *AuthPolicyReconciler) policyHosts(ap *api.AuthPolicy, targetNetworkObje return common.TargetHostnames(targetNetworkObject) } - uniqueHostnamesMap := make(map[string]interface{}) + uniqueHostnamesMap := make(map[string]any) for idx := range ap.Spec.AuthRules { if len(ap.Spec.AuthRules[idx].Hosts) == 0 { // When one of the rules does not have hosts, just return target hostnames diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index 8a23558ec..fa280d055 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -172,7 +172,7 @@ func (r *AuthPolicyReconciler) deleteResources(ctx context.Context, ap *api.Auth // remove direct back ref if targetNetworkObject != nil { - if err := r.deleteNetworkResourceDirectBackReference(ctx, ap, targetNetworkObject); err != nil { + if err := r.deleteNetworkResourceDirectBackReference(ctx, targetNetworkObject); err != nil { return err } } @@ -186,8 +186,8 @@ func (r *AuthPolicyReconciler) reconcileNetworkResourceDirectBackReference(ctx c return r.ReconcileTargetBackReference(ctx, client.ObjectKeyFromObject(ap), targetNetworkObject, common.AuthPolicyBackRefAnnotation) } -func (r *AuthPolicyReconciler) deleteNetworkResourceDirectBackReference(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error { - return r.DeleteTargetBackReference(ctx, client.ObjectKeyFromObject(ap), targetNetworkObject, common.AuthPolicyBackRefAnnotation) +func (r *AuthPolicyReconciler) deleteNetworkResourceDirectBackReference(ctx context.Context, targetNetworkObject client.Object) error { + return r.DeleteTargetBackReference(ctx, targetNetworkObject, common.AuthPolicyBackRefAnnotation) } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/authpolicy_controller_test.go b/controllers/authpolicy_controller_test.go index 6eda48086..74318cea2 100644 --- a/controllers/authpolicy_controller_test.go +++ b/controllers/authpolicy_controller_test.go @@ -428,9 +428,7 @@ func testBasicAuthScheme() kuadrantv1beta1.AuthSchemeSpec { }, }, Credentials: authorinov1beta1.Credentials{ - In: authorinov1beta1.Credentials_In( - "authorization_header", - ), + In: "authorization_header", KeySelector: "APIKEY", }, }, diff --git a/controllers/authpolicy_istio_authorization_policy.go b/controllers/authpolicy_istio_authorization_policy.go index 798841928..3706b30f2 100644 --- a/controllers/authpolicy_istio_authorization_policy.go +++ b/controllers/authpolicy_istio_authorization_policy.go @@ -6,6 +6,7 @@ import ( "reflect" "github.com/go-logr/logr" + "golang.org/x/exp/slices" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -19,9 +20,10 @@ import ( api "github.com/kuadrant/kuadrant-operator/api/v1beta1" "github.com/kuadrant/kuadrant-operator/pkg/common" "github.com/kuadrant/kuadrant-operator/pkg/reconcilers" + "k8s.io/utils/env" ) -var KuadrantExtAuthProviderName = common.FetchEnv("AUTH_PROVIDER", "kuadrant-authorization") +var KuadrantExtAuthProviderName = env.GetString("AUTH_PROVIDER", "kuadrant-authorization") // reconcileIstioAuthorizationPolicies translates and reconciles `AuthRules` into an Istio AuthorizationPoilcy containing them. func (r *AuthPolicyReconciler) reconcileIstioAuthorizationPolicies(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object, gwDiffObj *reconcilers.GatewayDiff) error { @@ -63,7 +65,7 @@ func (r *AuthPolicyReconciler) deleteIstioAuthorizationPolicies(ctx context.Cont } for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef { - listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(labels.Set(istioAuthorizationPolicyLabels(client.ObjectKeyFromObject(gw.Gateway), client.ObjectKeyFromObject(ap))))} + listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(istioAuthorizationPolicyLabels(client.ObjectKeyFromObject(gw.Gateway), client.ObjectKeyFromObject(ap)))} iapList := &istio.AuthorizationPolicyList{} if err := r.Client().List(ctx, iapList, listOptions); err != nil { return err @@ -71,7 +73,7 @@ func (r *AuthPolicyReconciler) deleteIstioAuthorizationPolicies(ctx context.Cont for _, iap := range iapList.Items { // it's OK to just go ahead and delete because we only create one IAP per target network object, - // and a network object can be target by no more than one AuthPolicy + // and a network object can be targeted by no more than one AuthPolicy if err := r.DeleteResource(ctx, iap); err != nil && !apierrors.IsNotFound(err) { logger.Error(err, "failed to delete IstioAuthorizationPolicy") return err @@ -155,9 +157,9 @@ func istioAuthorizationPolicyRules(authRules []api.AuthRule, targetHostnames []s for idx := range httpRouterules { toRules = append(toRules, &istiosecurity.Rule_To{ Operation: &istiosecurity.Operation{ - Hosts: common.SliceCopy(httpRouterules[idx].Hosts), - Methods: common.SliceCopy(httpRouterules[idx].Methods), - Paths: common.SliceCopy(httpRouterules[idx].Paths), + Hosts: slices.Clone(httpRouterules[idx].Hosts), + Methods: slices.Clone(httpRouterules[idx].Methods), + Paths: slices.Clone(httpRouterules[idx].Paths), }, }) } diff --git a/controllers/authpolicy_status.go b/controllers/authpolicy_status.go index 4cb4cc69c..9fc238bd1 100644 --- a/controllers/authpolicy_status.go +++ b/controllers/authpolicy_status.go @@ -7,9 +7,9 @@ import ( "github.com/go-logr/logr" authorinov1beta1 "github.com/kuadrant/authorino/api/v1beta1" kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" - "github.com/kuadrant/kuadrant-operator/pkg/common" + "golang.org/x/exp/slices" "k8s.io/apimachinery/pkg/api/errors" - meta "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -73,12 +73,12 @@ func (r *AuthPolicyReconciler) reconcileStatus(ctx context.Context, ap *kuadrant func (r *AuthPolicyReconciler) calculateStatus(ap *kuadrantv1beta1.AuthPolicy, specErr error, authConfigReady bool) *kuadrantv1beta1.AuthPolicyStatus { newStatus := &kuadrantv1beta1.AuthPolicyStatus{ - Conditions: common.CopyConditions(ap.Status.Conditions), + Conditions: slices.Clone(ap.Status.Conditions), ObservedGeneration: ap.Status.ObservedGeneration, } - targetNetworkObjectectKind := string(ap.Spec.TargetRef.Kind) - availableCond := r.availableCondition(targetNetworkObjectectKind, specErr, authConfigReady) + targetNetworkObjectKind := string(ap.Spec.TargetRef.Kind) + availableCond := r.availableCondition(targetNetworkObjectKind, specErr, authConfigReady) meta.SetStatusCondition(&newStatus.Conditions, *availableCond) @@ -86,7 +86,7 @@ func (r *AuthPolicyReconciler) calculateStatus(ap *kuadrantv1beta1.AuthPolicy, s } func (r *AuthPolicyReconciler) availableCondition(targetNetworkObjectectKind string, specErr error, authConfigReady bool) *metav1.Condition { - // Condition if there is not issue + // Condition if there is no issue cond := &metav1.Condition{ Type: APAvailableConditionType, Status: metav1.ConditionTrue, diff --git a/controllers/helper_test.go b/controllers/helper_test.go index edde09529..6ee41f06c 100644 --- a/controllers/helper_test.go +++ b/controllers/helper_test.go @@ -17,7 +17,7 @@ import ( . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - meta "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" diff --git a/controllers/kuadrant_controller.go b/controllers/kuadrant_controller.go index 43e313084..08a529c2c 100644 --- a/controllers/kuadrant_controller.go +++ b/controllers/kuadrant_controller.go @@ -21,6 +21,7 @@ import ( "encoding/json" corev1 "k8s.io/api/core/v1" + "k8s.io/utils/env" "github.com/go-logr/logr" authorinov1beta1 "github.com/kuadrant/authorino-operator/api/v1beta1" @@ -135,11 +136,7 @@ func (r *KuadrantReconciler) Reconcile(eventCtx context.Context, req ctrl.Reques logger.V(1).Error(gwErr, "Reconciling cluster gateways failed") } - specResult, specErr := r.reconcileSpec(ctx, kObj) - if specErr == nil && specResult.Requeue { - logger.V(1).Info("Reconciling spec not finished. Requeueing.") - return specResult, nil - } + specErr := r.reconcileSpec(ctx, kObj) statusResult, statusErr := r.reconcileStatus(ctx, kObj, specErr) @@ -356,32 +353,28 @@ func (r *KuadrantReconciler) registerServiceMeshMember(ctx context.Context, kObj return r.ReconcileResource(ctx, &maistrav1.ServiceMeshMember{}, member, reconcilers.CreateOnlyMutator) } -func (r *KuadrantReconciler) reconcileSpec(ctx context.Context, kObj *kuadrantv1beta1.Kuadrant) (ctrl.Result, error) { +func (r *KuadrantReconciler) reconcileSpec(ctx context.Context, kObj *kuadrantv1beta1.Kuadrant) error { if err := r.registerExternalAuthorizer(ctx, kObj); err != nil { - return ctrl.Result{}, err + return err } if err := r.reconcileLimitador(ctx, kObj); err != nil { - return ctrl.Result{}, err - } - - if err := r.reconcileAuthorino(ctx, kObj); err != nil { - return ctrl.Result{}, err + return err } - return ctrl.Result{}, nil + return r.reconcileAuthorino(ctx, kObj) } func controlPlaneProviderName() string { - return common.FetchEnv("ISTIOOPERATOR_NAME", "istiocontrolplane") + return env.GetString("ISTIOOPERATOR_NAME", "istiocontrolplane") } func controlPlaneConfigMapName() string { - return common.FetchEnv("ISTIOCONFIGMAP_NAME", "istio") + return env.GetString("ISTIOCONFIGMAP_NAME", "istio") } func controlPlaneProviderNamespace() string { - return common.FetchEnv("ISTIOOPERATOR_NAMESPACE", "istio-system") + return env.GetString("ISTIOOPERATOR_NAMESPACE", "istio-system") } func buildServiceMeshMember(kObj *kuadrantv1beta1.Kuadrant) *maistrav1.ServiceMeshMember { @@ -430,10 +423,7 @@ func (r *KuadrantReconciler) reconcileClusterGateways(ctx context.Context, kObj } } - if err := errGroup.Wait(); err != nil { - return err - } - return nil + return errGroup.Wait() } func (r *KuadrantReconciler) removeAnnotationFromGateways(ctx context.Context, kObj *kuadrantv1beta1.Kuadrant) error { @@ -461,10 +451,7 @@ func (r *KuadrantReconciler) removeAnnotationFromGateways(ctx context.Context, k }) } - if err := errGroup.Wait(); err != nil { - return err - } - return nil + return errGroup.Wait() } func (r *KuadrantReconciler) reconcileLimitador(ctx context.Context, kObj *kuadrantv1beta1.Kuadrant) error { diff --git a/controllers/kuadrant_status.go b/controllers/kuadrant_status.go index fe2f9d1d7..2ee1c1dfd 100644 --- a/controllers/kuadrant_status.go +++ b/controllers/kuadrant_status.go @@ -5,10 +5,11 @@ import ( "fmt" "github.com/go-logr/logr" + "golang.org/x/exp/slices" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - meta "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -64,7 +65,7 @@ func (r *KuadrantReconciler) reconcileStatus(ctx context.Context, kObj *kuadrant func (r *KuadrantReconciler) calculateStatus(ctx context.Context, kObj *kuadrantv1beta1.Kuadrant, specErr error) (*kuadrantv1beta1.KuadrantStatus, error) { newStatus := &kuadrantv1beta1.KuadrantStatus{ // Copy initial conditions. Otherwise, status will always be updated - Conditions: common.CopyConditions(kObj.Status.Conditions), + Conditions: slices.Clone(kObj.Status.Conditions), ObservedGeneration: kObj.Status.ObservedGeneration, } @@ -88,7 +89,7 @@ func (r *KuadrantReconciler) readyCondition(ctx context.Context, kObj *kuadrantv if specErr != nil { cond.Status = metav1.ConditionFalse - cond.Reason = "ReconcilliationError" + cond.Reason = "ReconciliationError" cond.Message = specErr.Error() return cond, nil } diff --git a/controllers/ratelimitpolicy_cluster_envoy_filter.go b/controllers/ratelimitpolicy_cluster_envoy_filter.go index 1157d76fe..e90d02599 100644 --- a/controllers/ratelimitpolicy_cluster_envoy_filter.go +++ b/controllers/ratelimitpolicy_cluster_envoy_filter.go @@ -6,6 +6,7 @@ import ( "github.com/go-logr/logr" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + "golang.org/x/exp/slices" istioapinetworkingv1alpha3 "istio.io/api/networking/v1alpha3" istioclientnetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3" "k8s.io/apimachinery/pkg/api/errors" @@ -50,7 +51,7 @@ func (r *RateLimitPolicyReconciler) reconcileRateLimitingClusterEnvoyFilter(ctx rlpRefs := gw.PolicyRefs() rlpKey := client.ObjectKeyFromObject(rlp) // Add the RLP key to the reference list. Only if it does not exist (it should not) - if !common.ContainsObjectKey(rlpRefs, rlpKey) { + if !slices.Contains(rlpRefs, rlpKey) { rlpRefs = append(gw.PolicyRefs(), rlpKey) } ef, err := r.gatewayRateLimitingClusterEnvoyFilter(ctx, gw.Gateway, rlpRefs) diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index 92331f248..690c3f2f2 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -211,7 +211,7 @@ func (r *RateLimitPolicyReconciler) deleteResources(ctx context.Context, rlp *ku // remove direct back ref if targetNetworkObject != nil { - if err := r.deleteNetworkResourceDirectBackReference(ctx, rlp, targetNetworkObject); err != nil { + if err := r.deleteNetworkResourceDirectBackReference(ctx, targetNetworkObject); err != nil { return err } } @@ -225,8 +225,8 @@ func (r *RateLimitPolicyReconciler) reconcileNetworkResourceDirectBackReference( return r.ReconcileTargetBackReference(ctx, client.ObjectKeyFromObject(policy), targetNetworkObject, common.RateLimitPolicyBackRefAnnotation) } -func (r *RateLimitPolicyReconciler) deleteNetworkResourceDirectBackReference(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, targetNetworkObject client.Object) error { - return r.DeleteTargetBackReference(ctx, client.ObjectKeyFromObject(rlp), targetNetworkObject, common.RateLimitPolicyBackRefAnnotation) +func (r *RateLimitPolicyReconciler) deleteNetworkResourceDirectBackReference(ctx context.Context, targetNetworkObject client.Object) error { + return r.DeleteTargetBackReference(ctx, targetNetworkObject, common.RateLimitPolicyBackRefAnnotation) } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go index 7c5b155c1..c1c6bd923 100644 --- a/controllers/ratelimitpolicy_controller_test.go +++ b/controllers/ratelimitpolicy_controller_test.go @@ -26,6 +26,7 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/rlptools" "github.com/kuadrant/kuadrant-operator/pkg/rlptools/wasm" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" + "k8s.io/utils/ptr" ) func testBuildBasicGateway(gwName, ns string) *gatewayapiv1beta1.Gateway { @@ -41,12 +42,12 @@ func testBuildBasicGateway(gwName, ns string) *gatewayapiv1beta1.Gateway { Annotations: map[string]string{"networking.istio.io/service-type": string(corev1.ServiceTypeClusterIP)}, }, Spec: gatewayapiv1beta1.GatewaySpec{ - GatewayClassName: gatewayapiv1beta1.ObjectName("istio"), + GatewayClassName: "istio", Listeners: []gatewayapiv1beta1.Listener{ { - Name: gatewayapiv1beta1.SectionName("default"), + Name: "default", Port: gatewayapiv1beta1.PortNumber(80), - Protocol: gatewayapiv1beta1.ProtocolType("HTTP"), + Protocol: "HTTP", }, }, }, @@ -69,7 +70,7 @@ func testBuildBasicHttpRoute(routeName, gwName, ns string, hostnames []string) * ParentRefs: []gatewayapiv1beta1.ParentReference{ { Name: gatewayapiv1beta1.ObjectName(gwName), - Namespace: common.Ptr(gatewayapiv1beta1.Namespace(ns)), + Namespace: ptr.To(gatewayapiv1beta1.Namespace(ns)), }, }, }, @@ -79,10 +80,10 @@ func testBuildBasicHttpRoute(routeName, gwName, ns string, hostnames []string) * Matches: []gatewayapiv1beta1.HTTPRouteMatch{ { Path: &gatewayapiv1beta1.HTTPPathMatch{ - Type: common.Ptr(gatewayapiv1beta1.PathMatchPathPrefix), - Value: common.Ptr("/toy"), + Type: ptr.To(gatewayapiv1beta1.PathMatchPathPrefix), + Value: ptr.To("/toy"), }, - Method: common.Ptr(gatewayapiv1beta1.HTTPMethod("GET")), + Method: ptr.To(gatewayapiv1beta1.HTTPMethod("GET")), }, }, }, @@ -283,17 +284,17 @@ var _ = Describe("RateLimitPolicy controller", func() { Matches: []gatewayapiv1beta1.HTTPRouteMatch{ { // get /toys* Path: &gatewayapiv1beta1.HTTPPathMatch{ - Type: common.Ptr(gatewayapiv1beta1.PathMatchPathPrefix), - Value: common.Ptr("/toys"), + Type: ptr.To(gatewayapiv1beta1.PathMatchPathPrefix), + Value: ptr.To("/toys"), }, - Method: common.Ptr(gatewayapiv1beta1.HTTPMethod("GET")), + Method: ptr.To(gatewayapiv1beta1.HTTPMethod("GET")), }, { // post /toys* Path: &gatewayapiv1beta1.HTTPPathMatch{ - Type: common.Ptr(gatewayapiv1beta1.PathMatchPathPrefix), - Value: common.Ptr("/toys"), + Type: ptr.To(gatewayapiv1beta1.PathMatchPathPrefix), + Value: ptr.To("/toys"), }, - Method: common.Ptr(gatewayapiv1beta1.HTTPMethod("POST")), + Method: ptr.To(gatewayapiv1beta1.HTTPMethod("POST")), }, }, }, @@ -301,8 +302,8 @@ var _ = Describe("RateLimitPolicy controller", func() { Matches: []gatewayapiv1beta1.HTTPRouteMatch{ { // /assets* Path: &gatewayapiv1beta1.HTTPPathMatch{ - Type: common.Ptr(gatewayapiv1beta1.PathMatchPathPrefix), - Value: common.Ptr("/assets"), + Type: ptr.To(gatewayapiv1beta1.PathMatchPathPrefix), + Value: ptr.To("/assets"), }, }, }, @@ -338,8 +339,8 @@ var _ = Describe("RateLimitPolicy controller", func() { Matches: []gatewayapiv1beta1.HTTPRouteMatch{ { Path: &gatewayapiv1beta1.HTTPPathMatch{ - Type: common.Ptr(gatewayapiv1beta1.PathMatchPathPrefix), - Value: common.Ptr("/toys"), + Type: ptr.To(gatewayapiv1beta1.PathMatchPathPrefix), + Value: ptr.To("/toys"), }, }, }, @@ -364,8 +365,8 @@ var _ = Describe("RateLimitPolicy controller", func() { Matches: []gatewayapiv1beta1.HTTPRouteMatch{ { Path: &gatewayapiv1beta1.HTTPPathMatch{ - Type: common.Ptr(gatewayapiv1beta1.PathMatchPathPrefix), - Value: common.Ptr("/assets"), + Type: ptr.To(gatewayapiv1beta1.PathMatchPathPrefix), + Value: ptr.To("/assets"), }, }, }, diff --git a/controllers/ratelimitpolicy_status.go b/controllers/ratelimitpolicy_status.go index f57717c76..14e3e079c 100644 --- a/controllers/ratelimitpolicy_status.go +++ b/controllers/ratelimitpolicy_status.go @@ -5,14 +5,14 @@ import ( "fmt" "github.com/go-logr/logr" + "golang.org/x/exp/slices" apierrors "k8s.io/apimachinery/pkg/api/errors" - meta "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/reconcile" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" - "github.com/kuadrant/kuadrant-operator/pkg/common" ) const ( @@ -58,7 +58,7 @@ func (r *RateLimitPolicyReconciler) reconcileStatus(ctx context.Context, rlp *ku func (r *RateLimitPolicyReconciler) calculateStatus(_ context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, specErr error) *kuadrantv1beta2.RateLimitPolicyStatus { newStatus := &kuadrantv1beta2.RateLimitPolicyStatus{ // Copy initial conditions. Otherwise, status will always be updated - Conditions: common.CopyConditions(rlp.Status.Conditions), + Conditions: slices.Clone(rlp.Status.Conditions), ObservedGeneration: rlp.Status.ObservedGeneration, } diff --git a/controllers/ratelimitpolicy_wasm_plugins.go b/controllers/ratelimitpolicy_wasm_plugins.go index 78cb994f3..093e24826 100644 --- a/controllers/ratelimitpolicy_wasm_plugins.go +++ b/controllers/ratelimitpolicy_wasm_plugins.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/go-logr/logr" + "golang.org/x/exp/slices" istioextensionsv1alpha1 "istio.io/api/extensions/v1alpha1" istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -57,7 +58,7 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context, rlpRefs := gw.PolicyRefs() rlpKey := client.ObjectKeyFromObject(rlp) // Add the RLP key to the reference list. Only if it does not exist (it should not) - if !common.ContainsObjectKey(rlpRefs, rlpKey) { + if !slices.Contains(rlpRefs, rlpKey) { rlpRefs = append(gw.PolicyRefs(), rlpKey) } wp, err := r.gatewayWASMPlugin(ctx, gw, rlpRefs) diff --git a/doc/development.md b/doc/development.md index 4b0874fc9..4582ce6f3 100644 --- a/doc/development.md +++ b/doc/development.md @@ -26,7 +26,7 @@ * [operator-sdk] version v1.28.1 * [kind] version v0.20.0 * [git][git_tool] -* [go] version 1.19+ +* [go] version 1.20+ * [kubernetes] version v1.19+ * [kubectl] version v1.19+ diff --git a/go.mod b/go.mod index f6260413b..2a67d9fff 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/kuadrant/kuadrant-operator -go 1.19 +go 1.20 require ( github.com/elliotchance/orderedmap/v2 v2.2.0 @@ -13,6 +13,7 @@ require ( github.com/onsi/ginkgo/v2 v2.7.0 github.com/onsi/gomega v1.24.2 go.uber.org/zap v1.24.0 + golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 golang.org/x/sync v0.1.0 google.golang.org/protobuf v1.31.0 gotest.tools v2.2.0+incompatible @@ -24,6 +25,7 @@ require ( k8s.io/apimachinery v0.26.1 k8s.io/client-go v0.26.1 k8s.io/klog/v2 v2.80.1 + k8s.io/utils v0.0.0-20230726121419-3b25d923346b sigs.k8s.io/controller-runtime v0.14.6 sigs.k8s.io/gateway-api v0.6.2 ) @@ -73,7 +75,7 @@ require ( go.uber.org/multierr v1.9.0 // indirect golang.org/x/net v0.9.0 // indirect golang.org/x/oauth2 v0.5.0 // indirect - golang.org/x/sys v0.7.0 // indirect + golang.org/x/sys v0.11.0 // indirect golang.org/x/term v0.7.0 // indirect golang.org/x/text v0.9.0 // indirect golang.org/x/time v0.3.0 // indirect @@ -89,7 +91,6 @@ require ( istio.io/pkg v0.0.0-20230523222708-7056be172a30 // indirect k8s.io/component-base v0.26.1 // indirect k8s.io/kube-openapi v0.0.0-20230109183929-3758b55a6596 // indirect - k8s.io/utils v0.0.0-20221128185143-99ec85e7a448 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect sigs.k8s.io/yaml v1.3.0 // indirect diff --git a/go.sum b/go.sum index 8a6a43fa3..8d7637171 100644 --- a/go.sum +++ b/go.sum @@ -263,7 +263,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.5.0 h1:U/0M97KRkSFvyD/3FSmdP5W5swImpNgle/EHFhOsQPE= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= -golang.org/x/exp v0.0.0-20230108222341-4b8118a2686a h1:tlXy25amD5A7gOfbXdqCGN5k8ESEed/Ee1E5RcrYnqU= +golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 h1:m64FZMko/V45gv0bNmrNYoDEq8U5YUhetc9cBWKS1TQ= +golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63/go.mod h1:0v4NqG35kSWCMzLaMeX+IQrlSnVE/bqGSyC2cz/9Le8= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= @@ -308,8 +309,8 @@ golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU= -golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM= +golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.7.0 h1:BEvjmm5fURWqcfbSKTdpkDXYBrUS1c0m8agp14W48vQ= golang.org/x/term v0.7.0/go.mod h1:P32HKFT3hSsZrRxla30E9HqToFYAQPCMs/zFMBUFqPY= @@ -423,8 +424,8 @@ k8s.io/klog/v2 v2.80.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= k8s.io/kube-openapi v0.0.0-20230109183929-3758b55a6596 h1:8cNCQs+WqqnSpZ7y0LMQPKD+RZUHU17VqLPMW3qxnxc= k8s.io/kube-openapi v0.0.0-20230109183929-3758b55a6596/go.mod h1:/BYxry62FuDzmI+i9B+X2pqfySRmSOW2ARmj5Zbqhj0= k8s.io/kubectl v0.26.0 h1:xmrzoKR9CyNdzxBmXV7jW9Ln8WMrwRK6hGbbf69o4T0= -k8s.io/utils v0.0.0-20221128185143-99ec85e7a448 h1:KTgPnR10d5zhztWptI952TNtt/4u5h3IzDXkdIMuo2Y= -k8s.io/utils v0.0.0-20221128185143-99ec85e7a448/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= +k8s.io/utils v0.0.0-20230726121419-3b25d923346b h1:sgn3ZU783SCgtaSJjpcVVlRqd6GSnlTLKgpAAttJvpI= +k8s.io/utils v0.0.0-20230726121419-3b25d923346b/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= sigs.k8s.io/controller-runtime v0.14.6 h1:oxstGVvXGNnMvY7TAESYk+lzr6S3V5VFxQ6d92KcwQA= sigs.k8s.io/controller-runtime v0.14.6/go.mod h1:WqIdsAY6JBsjfc/CqO0CORmNtoCtE4S6qbPc9s68h+0= sigs.k8s.io/gateway-api v0.6.2 h1:583XHiX2M2bKEA0SAdkoxL1nY73W1+/M+IAm8LJvbEA= diff --git a/main.go b/main.go index 877773e7f..59fd4fe85 100644 --- a/main.go +++ b/main.go @@ -23,6 +23,7 @@ import ( "runtime" authorinov1beta1 "github.com/kuadrant/authorino/api/v1beta1" + "k8s.io/utils/env" corev1 "k8s.io/api/core/v1" @@ -48,7 +49,6 @@ import ( kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2" "github.com/kuadrant/kuadrant-operator/controllers" - "github.com/kuadrant/kuadrant-operator/pkg/common" "github.com/kuadrant/kuadrant-operator/pkg/log" "github.com/kuadrant/kuadrant-operator/pkg/reconcilers" //+kubebuilder:scaffold:imports @@ -56,8 +56,8 @@ import ( var ( scheme = k8sruntime.NewScheme() - logLevel = common.FetchEnv("LOG_LEVEL", "info") - logMode = common.FetchEnv("LOG_MODE", "production") + logLevel = env.GetString("LOG_LEVEL", "info") + logMode = env.GetString("LOG_MODE", "production") ) func init() { diff --git a/pkg/common/apimachinery_status_conditions.go b/pkg/common/apimachinery_status_conditions.go index 4f7399c07..68ef12537 100644 --- a/pkg/common/apimachinery_status_conditions.go +++ b/pkg/common/apimachinery_status_conditions.go @@ -4,19 +4,13 @@ import ( "encoding/json" "sort" + "golang.org/x/exp/slices" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// CopyConditions copies the set of conditions -func CopyConditions(conditions []metav1.Condition) []metav1.Condition { - newConditions := make([]metav1.Condition, len(conditions)) - copy(newConditions, conditions) - return newConditions -} - // ConditionMarshal marshals the set of conditions as a JSON array, sorted by condition type. func ConditionMarshal(conditions []metav1.Condition) ([]byte, error) { - condCopy := CopyConditions(conditions) + condCopy := slices.Clone(conditions) sort.Slice(condCopy, func(a, b int) bool { return condCopy[a].Type < condCopy[b].Type }) diff --git a/pkg/common/apimachinery_status_conditions_test.go b/pkg/common/apimachinery_status_conditions_test.go index 15346f447..29c152358 100644 --- a/pkg/common/apimachinery_status_conditions_test.go +++ b/pkg/common/apimachinery_status_conditions_test.go @@ -10,113 +10,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestCopyConditions(t *testing.T) { - now := metav1.Now() - testCases := []struct { - name string - expectedConditions []metav1.Condition - }{ - { - name: "when multiple conditions then return copy", - expectedConditions: []metav1.Condition{ - { - Type: "Installed", - Status: metav1.ConditionTrue, - ObservedGeneration: 1, - LastTransitionTime: now, - Reason: "InstallSuccessful", - Message: "Object successfully installed", - }, - { - Type: "Reconciled", - Status: metav1.ConditionFalse, - ObservedGeneration: 0, - LastTransitionTime: now, - Reason: "ReconcileError", - Message: "Object failed to reconcile due to an error", - }, - { - Type: "Ready", - Status: metav1.ConditionFalse, - ObservedGeneration: 2, - LastTransitionTime: now, - Reason: "ComponentsNotReady", - Message: "Object components are not ready", - }, - { - Type: "Validated", - Status: metav1.ConditionUnknown, - ObservedGeneration: 0, - LastTransitionTime: now, - Reason: "ValidationError", - Message: "Object validation failed", - }, - }, - }, - { - name: "when one condition then return copy", - expectedConditions: []metav1.Condition{ - { - Type: "Validated", - Status: metav1.ConditionUnknown, - ObservedGeneration: 0, - LastTransitionTime: now, - Reason: "ValidationError", - Message: "Object validation failed", - }, - }, - }, - { - name: "when empty conditions slice return empty slice", - expectedConditions: []metav1.Condition{}, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(subT *testing.T) { - actualConditions := CopyConditions(tc.expectedConditions) - if diff := goCmp.Diff(tc.expectedConditions, actualConditions); diff != "" { - subT.Errorf("Conditions mismatch (-want +got):\n%s", diff) - } - }) - } -} - -func TestCopyConditions_OriginalConditionsNotModified(t *testing.T) { - now := metav1.Now() - expectedConditions := []metav1.Condition{ - { - Type: "Installed", - Status: metav1.ConditionTrue, - ObservedGeneration: 1, - LastTransitionTime: now, - Reason: "InstallSuccessful", - Message: "Object successfully installed", - }, - { - Type: "Reconciled", - Status: metav1.ConditionFalse, - ObservedGeneration: 0, - LastTransitionTime: now, - Reason: "ReconcileError", - Message: "Object failed to reconcile due to an error", - }, - } - - originalConditions := append([]metav1.Condition{}, expectedConditions...) - actualConditions := CopyConditions(expectedConditions) - - // Check that the copied conditions are equal to the original conditions - if diff := goCmp.Diff(expectedConditions, actualConditions); diff != "" { - t.Errorf("Conditions mismatch (-want +got):\n%s", diff) - } - - // Check that the original conditions were not modified - if diff := goCmp.Diff(originalConditions, expectedConditions); diff != "" { - t.Errorf("Original conditions were modified (-want +got):\n%s", diff) - } -} - func TestConditionMarshal(t *testing.T) { now := metav1.Now() nowUTC := now.UTC().Format(time.RFC3339) diff --git a/pkg/common/common.go b/pkg/common/common.go index b6f999d63..0f0e0d160 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -18,10 +18,9 @@ package common import ( "fmt" - "os" - "reflect" "strings" + "golang.org/x/exp/slices" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -30,7 +29,6 @@ import ( // TODO: move the const to a proper place, or get it from config const ( KuadrantRateLimitClusterName = "kuadrant-rate-limiting-service" - HTTPRouteKind = "HTTPRoute" RateLimitPoliciesBackRefAnnotation = "kuadrant.io/ratelimitpolicies" RateLimitPolicyBackRefAnnotation = "kuadrant.io/ratelimitpolicy" AuthPoliciesBackRefAnnotation = "kuadrant.io/authpolicies" @@ -47,31 +45,6 @@ type KuadrantPolicy interface { GetRulesHostnames() []string } -func Ptr[T any](t T) *T { - return &t -} - -// FetchEnv fetches the value of the environment variable with the specified key, -// or returns the default value if the variable is not found or has an empty value. -// If an error occurs during the lookup, the function returns the default value. -// The key and default value parameters must be valid strings. -func FetchEnv(key string, def string) string { - val, ok := os.LookupEnv(key) - if !ok { - return def - } - - return val -} - -// GetDefaultIfNil returns the value of a pointer argument, or a default value if the pointer is nil. -func GetDefaultIfNil[T any](val *T, def T) T { - if reflect.ValueOf(val).IsNil() { - return def - } - return *val -} - // GetEmptySliceIfNil returns a provided slice, or an empty slice of the same type if the input slice is nil. func GetEmptySliceIfNil[T any](val []T) []T { if val == nil { @@ -89,24 +62,13 @@ func NamespacedNameToObjectKey(namespacedName, defaultNamespace string) client.O return client.ObjectKey{Namespace: defaultNamespace, Name: namespacedName} } -// Contains checks if the given target string is present in the slice of strings 'slice'. -// It returns true if the target string is found in the slice, false otherwise. -func Contains[T comparable](slice []T, target T) bool { - for idx := range slice { - if slice[idx] == target { - return true - } - } - return false -} - // SameElements checks if the two slices contain the exact same elements. Order does not matter. func SameElements[T comparable](s1, s2 []T) bool { if len(s1) != len(s2) { return false } for _, v := range s1 { - if !Contains(s2, v) { + if !slices.Contains(s2, v) { return false } } @@ -115,7 +77,7 @@ func SameElements[T comparable](s1, s2 []T) bool { func Intersect[T comparable](slice1, slice2 []T) bool { for _, item := range slice1 { - if Contains(slice2, item) { + if slices.Contains(slice2, item) { return true } } @@ -131,7 +93,7 @@ func Intersection[T comparable](slice1, slice2 []T) []T { } var result []T for _, item := range smallerSlice { - if Contains(largerSlice, item) { + if slices.Contains(largerSlice, item) { result = append(result, item) } } @@ -167,27 +129,6 @@ func Filter[T any](slice []T, f func(T) bool) []T { return arr } -// SliceCopy copies the elements from the input slice into the output slice, and returns the output slice. -func SliceCopy[T any](s1 []T) []T { - s2 := make([]T, len(s1)) - copy(s2, s1) - return s2 -} - -// ReverseSlice creates a reversed copy of the input slice. -func ReverseSlice[T any](input []T) []T { - inputLen := len(input) - output := make([]T, inputLen) - - for i, n := range input { - j := inputLen - i - 1 - - output[j] = n - } - - return output -} - // MergeMapStringString Merge desired into existing. // Not Thread-Safe. Does it matter? func MergeMapStringString(existing *map[string]string, desired map[string]string) bool { @@ -238,7 +179,7 @@ func UnMarshallObjectKey(keyStr string) (client.ObjectKey, error) { return client.ObjectKey{Namespace: keyStr[:namespaceEndIndex], Name: keyStr[namespaceEndIndex+1:]}, nil } -// HostnamesToStrings converts []gatewayapi_v1alpha2.Hostname to []string +// HostnamesToStrings converts []gatewayapiv1beta1.Hostname to []string func HostnamesToStrings(hostnames []gatewayapiv1beta1.Hostname) []string { return Map(hostnames, func(hostname gatewayapiv1beta1.Hostname) string { return string(hostname) @@ -249,8 +190,8 @@ func HostnamesToStrings(hostnames []gatewayapiv1beta1.Hostname) []string { // is a subset of at least one of the domains. // Domains and subdomains may be prefixed with a wildcard label (*.). // The wildcard label must appear by itself as the first label. -// When one of the subdomains is not a subset of any of the domains, it returns false and -// the subdomain not being subset of any of the domains +// When one of the subdomains is not a subset of the domains, it returns false and +// the subdomain not being subset of the domains func ValidSubdomains(domains, subdomains []string) (bool, string) { for _, subdomain := range subdomains { validSubdomain := false diff --git a/pkg/common/common_test.go b/pkg/common/common_test.go index 94d01fc69..45f6b56cb 100644 --- a/pkg/common/common_test.go +++ b/pkg/common/common_test.go @@ -4,7 +4,6 @@ package common import ( "fmt" - "os" "reflect" "testing" @@ -74,78 +73,6 @@ func TestFind(t *testing.T) { } } -func TestFetchEnv(t *testing.T) { - t.Run("when env var exists & has a value different from the default value then return the env var value", func(t *testing.T) { - key := "LOG_LEVEL" - defaultVal := "info" - val := "debug" - os.Setenv(key, val) - defer os.Unsetenv(key) - - result := FetchEnv(key, defaultVal) - - if result != val { - t.Errorf("Expected %v, but got %v", val, result) - } - }) - t.Run("when env var exists but has an empty value then return the empty value", func(t *testing.T) { - key := "LOG_MODE" - defaultVal := "production" - val := "" - os.Setenv(key, val) - defer os.Unsetenv(key) - - result := FetchEnv(key, defaultVal) - - if result != val { - t.Errorf("Expected %v, but got %v", val, result) - } - }) - t.Run("when env var does not exist & the default value is used then return the default value", func(t *testing.T) { - key := "LOG_MODE" - defaultVal := "production" - - result := FetchEnv(key, defaultVal) - - if result != defaultVal { - t.Errorf("Expected %v, but got %v", defaultVal, result) - } - }) - t.Run("when default value is an empty string then return an empty string", func(t *testing.T) { - key := "LOG_MODE" - defaultVal := "" - - result := FetchEnv(key, defaultVal) - - if result != defaultVal { - t.Errorf("Expected %v, but got %v", defaultVal, result) - } - }) -} - -func TestGetDefaultIfNil(t *testing.T) { - t.Run("when value is non-nil pointer type and default value is provided then return value", func(t *testing.T) { - val := "test" - def := "default" - - result := GetDefaultIfNil(&val, def) - - if result != val { - t.Errorf("Expected %v, but got %v", val, result) - } - }) - t.Run("when value is nil pointer type and default value is provided then return default value", func(t *testing.T) { - var val *string - def := "default" - - result := GetDefaultIfNil(val, def) - - if result != def { - t.Errorf("Expected %v, but got %v", def, result) - } - }) -} - func TestGetEmptySliceIfNil(t *testing.T) { t.Run("when a non-nil slice is provided then return same slice", func(t *testing.T) { value := []int{1, 2, 3} @@ -208,90 +135,6 @@ func TestNamespacedNameToObjectKey(t *testing.T) { }) } -func TestContains(t *testing.T) { - testCases := []struct { - name string - slice []string - target string - expected bool - }{ - { - name: "when slice has one target item then return true", - slice: []string{"test-gw"}, - target: "test-gw", - expected: true, - }, - { - name: "when slice is empty then return false", - slice: []string{}, - target: "test-gw", - expected: false, - }, - { - name: "when target is in a slice then return true", - slice: []string{"test-gw1", "test-gw2", "test-gw3"}, - target: "test-gw2", - expected: true, - }, - { - name: "when no target in a slice then return false", - slice: []string{"test-gw1", "test-gw2", "test-gw3"}, - target: "test-gw4", - expected: false, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - if Contains(tc.slice, tc.target) != tc.expected { - t.Errorf("when slice=%v and target=%s, expected=%v, but got=%v", tc.slice, tc.target, tc.expected, !tc.expected) - } - }) - } -} - -func TestContainsWithInts(t *testing.T) { - testCases := []struct { - name string - slice []int - target int - expected bool - }{ - { - name: "when slice has one target item then return true", - slice: []int{1}, - target: 1, - expected: true, - }, - { - name: "when slice is empty then return false", - slice: []int{}, - target: 2, - expected: false, - }, - { - name: "when target is in a slice then return true", - slice: []int{1, 2, 3}, - target: 2, - expected: true, - }, - { - name: "when no target in a slice then return false", - slice: []int{1, 2, 3}, - target: 4, - expected: false, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - if Contains(tc.slice, tc.target) != tc.expected { - t.Errorf("when slice=%v and target=%d, expected=%v, but got=%v", tc.slice, tc.target, tc.expected, !tc.expected) - } - }) - } -} - func TestSameElements(t *testing.T) { testCases := []struct { name string @@ -550,81 +393,6 @@ func TestMap(t *testing.T) { }) } -func TestSliceCopy(t *testing.T) { - input1 := []int{1, 2, 3} - expected1 := []int{1, 2, 3} - output1 := SliceCopy(input1) - t.Run("when given slice of integers then return a copy of the input slice", func(t *testing.T) { - if !reflect.DeepEqual(output1, expected1) { - t.Errorf("SliceCopy(%v) = %v; expected %v", input1, output1, expected1) - } - }) - - input2 := []string{"foo", "bar", "baz"} - expected2 := []string{"foo", "bar", "baz"} - output2 := SliceCopy(input2) - t.Run("when given slice of strings then return a copy of the input slice", func(t *testing.T) { - if !reflect.DeepEqual(output2, expected2) { - t.Errorf("SliceCopy(%v) = %v; expected %v", input2, output2, expected2) - } - }) - - type person struct { - name string - age int - } - input3 := []person{{"Artem", 65}, {"DD", 18}, {"Charlie", 23}} - expected3 := []person{{"Artem", 65}, {"DD", 18}, {"Charlie", 23}} - output3 := SliceCopy(input3) - t.Run("when given slice of structs then return a copy of the input slice", func(t *testing.T) { - if !reflect.DeepEqual(output3, expected3) { - t.Errorf("SliceCopy(%v) = %v; expected %v", input3, output3, expected3) - } - }) - - input4 := []int{1, 2, 3} - expected4 := []int{1, 2, 3} - output4 := SliceCopy(input4) - t.Run("when modifying the original input slice then does not affect the returned copy", func(t *testing.T) { - if !reflect.DeepEqual(output4, expected4) { - t.Errorf("SliceCopy(%v) = %v; expected %v", input4, output4, expected4) - } - input4[0] = 4 - if reflect.DeepEqual(output4, input4) { - t.Errorf("modifying the original input slice should not change the output slice") - } - }) -} - -func TestReverseSlice(t *testing.T) { - input1 := []int{1, 2, 3} - expected1 := []int{3, 2, 1} - output1 := ReverseSlice(input1) - t.Run("when given slice of integers then return reversed copy of the input slice", func(t *testing.T) { - if !reflect.DeepEqual(output1, expected1) { - t.Errorf("ReverseSlice(%v) = %v; expected %v", input1, output1, expected1) - } - }) - - input2 := []string{"foo", "bar", "baz"} - expected2 := []string{"baz", "bar", "foo"} - output2 := ReverseSlice(input2) - t.Run("when given slice of strings then return reversed copy of the input slice", func(t *testing.T) { - if !reflect.DeepEqual(output2, expected2) { - t.Errorf("ReverseSlice(%v) = %v; expected %v", input2, output2, expected2) - } - }) - - input3 := []int{} - expected3 := []int{} - output3 := ReverseSlice(input3) - t.Run("when given an empty slice then return empty slice", func(t *testing.T) { - if !reflect.DeepEqual(output3, expected3) { - t.Errorf("ReverseSlice(%v) = %v; expected %v", input3, output3, expected3) - } - }) -} - func TestMergeMapStringString(t *testing.T) { testCases := []struct { name string diff --git a/pkg/common/gatewayapi_utils.go b/pkg/common/gatewayapi_utils.go index ccb743336..3f842dc26 100644 --- a/pkg/common/gatewayapi_utils.go +++ b/pkg/common/gatewayapi_utils.go @@ -7,9 +7,11 @@ import ( "reflect" "strings" + "golang.org/x/exp/slices" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -204,12 +206,12 @@ func HTTPMethodToString(method *gatewayapiv1beta1.HTTPMethod) string { func GetKuadrantNamespaceFromPolicyTargetRef(ctx context.Context, cli client.Client, policy KuadrantPolicy) (string, error) { targetRef := policy.GetTargetRef() - gwNamespacedName := types.NamespacedName{Namespace: string(GetDefaultIfNil(targetRef.Namespace, policy.GetWrappedNamespace())), Name: string(targetRef.Name)} + gwNamespacedName := types.NamespacedName{Namespace: string(ptr.Deref(targetRef.Namespace, policy.GetWrappedNamespace())), Name: string(targetRef.Name)} if IsTargetRefHTTPRoute(targetRef) { route := &gatewayapiv1beta1.HTTPRoute{} if err := cli.Get( ctx, - types.NamespacedName{Namespace: string(GetDefaultIfNil(targetRef.Namespace, policy.GetWrappedNamespace())), Name: string(targetRef.Name)}, + types.NamespacedName{Namespace: string(ptr.Deref(targetRef.Namespace, policy.GetWrappedNamespace())), Name: string(targetRef.Name)}, route, ); err != nil { return "", err @@ -316,7 +318,7 @@ func GatewaysMissingPolicyRef(gwList *gatewayapiv1beta1.GatewayList, policyKey c for i := range gwList.Items { gateway := gwList.Items[i] gw := GatewayWrapper{&gateway, config} - if ContainsObjectKey(policyGwKeys, client.ObjectKeyFromObject(&gateway)) && !gw.ContainsPolicy(policyKey) { + if slices.Contains(policyGwKeys, client.ObjectKeyFromObject(&gateway)) && !gw.ContainsPolicy(policyKey) { gateways = append(gateways, gw) } } @@ -329,7 +331,7 @@ func GatewaysWithValidPolicyRef(gwList *gatewayapiv1beta1.GatewayList, policyKey for i := range gwList.Items { gateway := gwList.Items[i] gw := GatewayWrapper{&gateway, config} - if ContainsObjectKey(policyGwKeys, client.ObjectKeyFromObject(&gateway)) && gw.ContainsPolicy(policyKey) { + if slices.Contains(policyGwKeys, client.ObjectKeyFromObject(&gateway)) && gw.ContainsPolicy(policyKey) { gateways = append(gateways, gw) } } @@ -342,7 +344,7 @@ func GatewaysWithInvalidPolicyRef(gwList *gatewayapiv1beta1.GatewayList, policyK for i := range gwList.Items { gateway := gwList.Items[i] gw := GatewayWrapper{&gateway, config} - if !ContainsObjectKey(policyGwKeys, client.ObjectKeyFromObject(&gateway)) && gw.ContainsPolicy(policyKey) { + if !slices.Contains(policyGwKeys, client.ObjectKeyFromObject(&gateway)) && gw.ContainsPolicy(policyKey) { gateways = append(gateways, gw) } } @@ -403,7 +405,7 @@ func (g GatewayWrapper) ContainsPolicy(policyKey client.ObjectKey) bool { return false } - return ContainsObjectKey(refs, policyKey) + return slices.Contains(refs, policyKey) } // AddPolicy tries to add a policy to the existing ref list. @@ -434,7 +436,7 @@ func (g GatewayWrapper) AddPolicy(policyKey client.ObjectKey) bool { return false } - if ContainsObjectKey(refs, policyKey) { + if slices.Contains(refs, policyKey) { return false } diff --git a/pkg/common/gatewayapi_utils_test.go b/pkg/common/gatewayapi_utils_test.go index 1f0668c22..5309baeba 100644 --- a/pkg/common/gatewayapi_utils_test.go +++ b/pkg/common/gatewayapi_utils_test.go @@ -8,6 +8,7 @@ import ( "reflect" "testing" + "golang.org/x/exp/slices" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -713,13 +714,13 @@ func TestGatewaysMissingPolicyRef(t *testing.T) { {Namespace: "gw-ns", Name: "gw-3"}, }, policyRefConfig), gwName) - if Contains(gws, "gw-1") { + if slices.Contains(gws, "gw-1") { t.Error("gateway expected not to be listed as missing policy ref") } - if Contains(gws, "gw-2") { + if slices.Contains(gws, "gw-2") { t.Error("gateway expected not to be listed as missing policy ref") } - if !Contains(gws, "gw-3") { + if !slices.Contains(gws, "gw-3") { t.Error("gateway expected to be listed as missing policy ref") } @@ -727,13 +728,13 @@ func TestGatewaysMissingPolicyRef(t *testing.T) { {Namespace: "gw-ns", Name: "gw-1"}, }, policyRefConfig), gwName) - if Contains(gws, "gw-1") { + if slices.Contains(gws, "gw-1") { t.Error("gateway expected not to be listed as missing policy ref") } - if Contains(gws, "gw-2") { + if slices.Contains(gws, "gw-2") { t.Error("gateway expected not to be listed as missing policy ref") } - if Contains(gws, "gw-3") { + if slices.Contains(gws, "gw-3") { t.Error("gateway expected not to be listed as missing policy ref") } @@ -742,13 +743,13 @@ func TestGatewaysMissingPolicyRef(t *testing.T) { {Namespace: "gw-ns", Name: "gw-3"}, }, policyRefConfig), gwName) - if !Contains(gws, "gw-1") { + if !slices.Contains(gws, "gw-1") { t.Error("gateway expected to be listed as missing policy ref") } - if Contains(gws, "gw-2") { + if slices.Contains(gws, "gw-2") { t.Error("gateway expected not to be listed as missing policy ref") } - if !Contains(gws, "gw-3") { + if !slices.Contains(gws, "gw-3") { t.Error("gateway expected to be listed as missing policy ref") } } @@ -788,13 +789,13 @@ func TestGatewaysWithValidPolicyRef(t *testing.T) { {Namespace: "gw-ns", Name: "gw-3"}, }, policyRefConfig), gwName) - if Contains(gws, "gw-1") { + if slices.Contains(gws, "gw-1") { t.Error("gateway expected not to be listed as with valid policy ref") } - if !Contains(gws, "gw-2") { + if !slices.Contains(gws, "gw-2") { t.Error("gateway expected to be listed as with valid policy ref") } - if Contains(gws, "gw-3") { + if slices.Contains(gws, "gw-3") { t.Error("gateway expected not to be listed as with valid policy ref") } @@ -802,13 +803,13 @@ func TestGatewaysWithValidPolicyRef(t *testing.T) { {Namespace: "gw-ns", Name: "gw-1"}, }, policyRefConfig), gwName) - if !Contains(gws, "gw-1") { + if !slices.Contains(gws, "gw-1") { t.Error("gateway expected to be listed as with valid policy ref") } - if Contains(gws, "gw-2") { + if slices.Contains(gws, "gw-2") { t.Error("gateway expected not to be listed as with valid policy ref") } - if Contains(gws, "gw-3") { + if slices.Contains(gws, "gw-3") { t.Error("gateway expected not to be listed as with valid policy ref") } @@ -817,13 +818,13 @@ func TestGatewaysWithValidPolicyRef(t *testing.T) { {Namespace: "gw-ns", Name: "gw-3"}, }, policyRefConfig), gwName) - if Contains(gws, "gw-1") { + if slices.Contains(gws, "gw-1") { t.Error("gateway expected not to be listed as with valid policy ref") } - if Contains(gws, "gw-2") { + if slices.Contains(gws, "gw-2") { t.Error("gateway expected not to be listed as with valid policy ref") } - if Contains(gws, "gw-3") { + if slices.Contains(gws, "gw-3") { t.Error("gateway expected not to be listed as with valid policy ref") } } @@ -863,13 +864,13 @@ func TestGatewaysWithInvalidPolicyRef(t *testing.T) { {Namespace: "gw-ns", Name: "gw-3"}, }, policyRefConfig), gwName) - if !Contains(gws, "gw-1") { + if !slices.Contains(gws, "gw-1") { t.Error("gateway expected to be listed as with invalid policy ref") } - if Contains(gws, "gw-2") { + if slices.Contains(gws, "gw-2") { t.Error("gateway expected not to be listed as with invalid policy ref") } - if Contains(gws, "gw-3") { + if slices.Contains(gws, "gw-3") { t.Error("gateway expected not to be listed as with invalid policy ref") } @@ -877,13 +878,13 @@ func TestGatewaysWithInvalidPolicyRef(t *testing.T) { {Namespace: "gw-ns", Name: "gw-1"}, }, policyRefConfig), gwName) - if Contains(gws, "gw-1") { + if slices.Contains(gws, "gw-1") { t.Error("gateway expected not to be listed as with invalid policy ref") } - if Contains(gws, "gw-2") { + if slices.Contains(gws, "gw-2") { t.Error("gateway expected not to be listed as with invalid policy ref") } - if Contains(gws, "gw-3") { + if slices.Contains(gws, "gw-3") { t.Error("gateway expected not to be listed as with invalid policy ref") } @@ -892,13 +893,13 @@ func TestGatewaysWithInvalidPolicyRef(t *testing.T) { {Namespace: "gw-ns", Name: "gw-3"}, }, policyRefConfig), gwName) - if Contains(gws, "gw-1") { + if slices.Contains(gws, "gw-1") { t.Error("gateway expected not to be listed as with invalid policy ref") } - if Contains(gws, "gw-2") { + if slices.Contains(gws, "gw-2") { t.Error("gateway expected not to be listed as with invalid policy ref") } - if Contains(gws, "gw-3") { + if slices.Contains(gws, "gw-3") { t.Error("gateway expected not to be listed as with invalid policy ref") } } @@ -931,10 +932,10 @@ func TestGatewayWrapperPolicyRefs(t *testing.T) { PolicyRefsConfig: &KuadrantRateLimitPolicyRefsConfig{}, } refs := Map(gw.PolicyRefs(), func(ref k8stypes.NamespacedName) string { return ref.String() }) - if !Contains(refs, "app-ns/policy-1") { + if !slices.Contains(refs, "app-ns/policy-1") { t.Error("GatewayWrapper.PolicyRefs() should contain app-ns/policy-1") } - if !Contains(refs, "app-ns/policy-2") { + if !slices.Contains(refs, "app-ns/policy-2") { t.Error("GatewayWrapper.PolicyRefs() should contain app-ns/policy-2") } if len(refs) != 2 { @@ -1032,7 +1033,7 @@ func TestGatewayHostnames(t *testing.T) { PolicyRefsConfig: &KuadrantRateLimitPolicyRefsConfig{}, } hostnames := gw.Hostnames() - if !Contains(hostnames, "toystore.com") { + if !slices.Contains(hostnames, "toystore.com") { t.Error("GatewayWrapper.Hostnames() expected to contain 'toystore.com'") } if len(hostnames) != 1 { @@ -1095,7 +1096,7 @@ func TestGetGatewayWorkloadSelector(t *testing.T) { scheme := runtime.NewScheme() _ = corev1.AddToScheme(scheme) _ = gatewayapiv1beta1.AddToScheme(scheme) - k8sClient := fake.NewFakeClientWithScheme(scheme, gateway, service) + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(gateway, service).Build() var selector map[string]string var err error @@ -1136,7 +1137,7 @@ func TestGetGatewayWorkloadSelectorWithoutHostnameAddress(t *testing.T) { scheme := runtime.NewScheme() _ = corev1.AddToScheme(scheme) _ = gatewayapiv1beta1.AddToScheme(scheme) - k8sClient := fake.NewFakeClientWithScheme(scheme, gateway, service) + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(gateway, service).Build() var selector map[string]string var err error @@ -1157,7 +1158,7 @@ func (p *FakePolicy) GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference { } func (p *FakePolicy) GetWrappedNamespace() gatewayapiv1beta1.Namespace { - return gatewayapiv1beta1.Namespace("") + return "" } func (p *FakePolicy) GetRulesHostnames() []string { @@ -1193,9 +1194,7 @@ func TestValidateHierarchicalRules(t *testing.T) { ) if err := ValidateHierarchicalRules(&policy2, gateway); err.Error() != expectedError.Error() { - t.Fatal("the error message does not match the expected error one") - fmt.Println(expectedError.Error()) - fmt.Println(err.Error()) + t.Fatal("the error message does not match the expected error one", expectedError.Error(), err.Error()) } } diff --git a/pkg/common/istio_utils_test.go b/pkg/common/istio_utils_test.go index 3744d04ac..a35a8677c 100644 --- a/pkg/common/istio_utils_test.go +++ b/pkg/common/istio_utils_test.go @@ -1,5 +1,4 @@ //go:build unit -// +build unit package common diff --git a/pkg/common/k8s_utils.go b/pkg/common/k8s_utils.go index 03f716db3..563e8300f 100644 --- a/pkg/common/k8s_utils.go +++ b/pkg/common/k8s_utils.go @@ -31,8 +31,7 @@ import ( ) const ( - DeleteTagAnnotation = "kuadrant.io/delete" - ReadyStatusConditionType = "Ready" + DeleteTagAnnotation = "kuadrant.io/delete" ) // ObjectInfo generates a string representation of the provided Kubernetes object, including its kind and name. @@ -80,9 +79,7 @@ func IsObjectTaggedToDelete(obj client.Object) bool { // condition type. func StatusConditionsMarshalJSON(input []metav1.Condition) ([]byte, error) { conds := make([]metav1.Condition, 0, len(input)) - for idx := range input { - conds = append(conds, input[idx]) - } + conds = append(conds, input...) sort.Slice(conds, func(a, b int) bool { return conds[a].Type < conds[b].Type @@ -170,16 +167,6 @@ func ObjectKeyListDifference(a, b []client.ObjectKey) []client.ObjectKey { return result } -// ContainsObjectKey tells whether a contains x -func ContainsObjectKey(a []client.ObjectKey, x client.ObjectKey) bool { - for _, n := range a { - if x == n { - return true - } - } - return false -} - // FindObjectKey returns the smallest index i at which x == a[i], // or len(a) if there is no such index. func FindObjectKey(a []client.ObjectKey, x client.ObjectKey) int { diff --git a/pkg/common/k8s_utils_test.go b/pkg/common/k8s_utils_test.go index 18e03b81b..f14adc651 100644 --- a/pkg/common/k8s_utils_test.go +++ b/pkg/common/k8s_utils_test.go @@ -1,5 +1,4 @@ //go:build unit -// +build unit package common @@ -732,48 +731,6 @@ func TestGetServicePortNumber(t *testing.T) { } } -func TestContainsObjectKey(t *testing.T) { - key1 := client.ObjectKey{Namespace: "ns1", Name: "obj1"} - key2 := client.ObjectKey{Namespace: "ns2", Name: "obj2"} - key3 := client.ObjectKey{Namespace: "ns3", Name: "obj3"} - key4 := client.ObjectKey{Namespace: "ns4", Name: "obj4"} - - testCases := []struct { - name string - list []client.ObjectKey - key client.ObjectKey - expected bool - }{ - { - name: "when list contains key then return true", - list: []client.ObjectKey{key1, key2, key3}, - key: key2, - expected: true, - }, - { - name: "when list does not contain key then return false", - list: []client.ObjectKey{key1, key2, key3}, - key: key4, - expected: false, - }, - { - name: "when list is empty then return false", - list: []client.ObjectKey{}, - key: key4, - expected: false, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - result := ContainsObjectKey(tc.list, tc.key) - if result != tc.expected { - t.Errorf("unexpected result: got %t, want %t", result, tc.expected) - } - }) - } -} - func TestFindObjectKey(t *testing.T) { key1 := client.ObjectKey{Namespace: "ns1", Name: "obj1"} key2 := client.ObjectKey{Namespace: "ns2", Name: "obj2"} diff --git a/pkg/common/mesh_config_test.go b/pkg/common/mesh_config_test.go index d0e8a8962..836e4fb59 100644 --- a/pkg/common/mesh_config_test.go +++ b/pkg/common/mesh_config_test.go @@ -1,5 +1,4 @@ //go:build unit -// +build unit package common diff --git a/pkg/common/yaml_decoder_test.go b/pkg/common/yaml_decoder_test.go index 8bf5307fa..f77c54752 100644 --- a/pkg/common/yaml_decoder_test.go +++ b/pkg/common/yaml_decoder_test.go @@ -1,5 +1,4 @@ //go:build unit -// +build unit package common diff --git a/pkg/istio/envoy_filters.go b/pkg/istio/envoy_filters.go index 31ead0102..dc8741fb6 100644 --- a/pkg/istio/envoy_filters.go +++ b/pkg/istio/envoy_filters.go @@ -15,23 +15,23 @@ import ( // Note: This should be removed once the mentioned issue is fixed but that will take some time. func LimitadorClusterPatch(limitadorSvcHost string, limitadorGRPCPort int) ([]*istioapiv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch, error) { // The patch defines the rate_limit_cluster, which provides the endpoint location of the external rate limit service. - patchUnstructured := map[string]interface{}{ + patchUnstructured := map[string]any{ "operation": "ADD", - "value": map[string]interface{}{ + "value": map[string]any{ "name": common.KuadrantRateLimitClusterName, "type": "STRICT_DNS", "connect_timeout": "1s", "lb_policy": "ROUND_ROBIN", - "http2_protocol_options": map[string]interface{}{}, - "load_assignment": map[string]interface{}{ + "http2_protocol_options": map[string]any{}, + "load_assignment": map[string]any{ "cluster_name": common.KuadrantRateLimitClusterName, - "endpoints": []map[string]interface{}{ + "endpoints": []map[string]any{ { - "lb_endpoints": []map[string]interface{}{ + "lb_endpoints": []map[string]any{ { - "endpoint": map[string]interface{}{ - "address": map[string]interface{}{ - "socket_address": map[string]interface{}{ + "endpoint": map[string]any{ + "address": map[string]any{ + "socket_address": map[string]any{ "address": limitadorSvcHost, "port_value": limitadorGRPCPort, }, diff --git a/pkg/istio/mesh_config.go b/pkg/istio/mesh_config.go index 0cd23eb19..2e012c9d5 100644 --- a/pkg/istio/mesh_config.go +++ b/pkg/istio/mesh_config.go @@ -127,10 +127,8 @@ func (w *OSSMControlPlaneWrapper) SetMeshConfig(config *istiomeshv1alpha1.MeshCo if err != nil { return err } - if err := w.config.Spec.TechPreview.SetField("meshConfig", meshConfigStruct.AsMap()); err != nil { - return err - } - return nil + + return w.config.Spec.TechPreview.SetField("meshConfig", meshConfigStruct.AsMap()) } // meshConfigFromStruct Builds the Istio/OSSM MeshConfig from a compatible structure: diff --git a/pkg/istio/mesh_config_test.go b/pkg/istio/mesh_config_test.go index 916c7397f..f75cf714b 100644 --- a/pkg/istio/mesh_config_test.go +++ b/pkg/istio/mesh_config_test.go @@ -1,5 +1,4 @@ //go:build unit -// +build unit package istio @@ -172,7 +171,8 @@ func TestOSSMControlPlaneWrapper_GetConfigObject(t *testing.T) { func TestOSSMControlPlaneWrapper_GetMeshConfig(t *testing.T) { ossmControlPlane := &maistrav2.ServiceMeshControlPlane{} ossmControlPlane.Spec.TechPreview = maistrav1.NewHelmValues(nil) - ossmControlPlane.Spec.TechPreview.SetField("meshConfig", getStubbedMeshConfigStruct().AsMap()) + err := ossmControlPlane.Spec.TechPreview.SetField("meshConfig", getStubbedMeshConfigStruct().AsMap()) + assert.NilError(t, err) wrapper := NewOSSMControlPlaneWrapper(ossmControlPlane) meshConfig, _ := wrapper.GetMeshConfig() @@ -185,12 +185,13 @@ func TestOSSMControlPlaneWrapper_SetMeshConfig(t *testing.T) { ossmControlPlane := &maistrav2.ServiceMeshControlPlane{} ossmControlPlane.Spec.TechPreview = maistrav1.NewHelmValues(nil) emptyConfig := &structpb.Struct{} - ossmControlPlane.Spec.TechPreview.SetField("meshConfig", emptyConfig.AsMap()) + err := ossmControlPlane.Spec.TechPreview.SetField("meshConfig", emptyConfig.AsMap()) + assert.NilError(t, err) wrapper := NewOSSMControlPlaneWrapper(ossmControlPlane) stubbedMeshConfig := getStubbedMeshConfig() - err := wrapper.SetMeshConfig(stubbedMeshConfig) + err = wrapper.SetMeshConfig(stubbedMeshConfig) assert.NilError(t, err) meshConfig, _ := wrapper.GetMeshConfig() diff --git a/pkg/log/log_test.go b/pkg/log/log_test.go index b9ef8551a..a75171b03 100644 --- a/pkg/log/log_test.go +++ b/pkg/log/log_test.go @@ -1,5 +1,4 @@ //go:build unit -// +build unit /* Copyright 2021 Red Hat, Inc. diff --git a/pkg/reconcilers/base_reconciler.go b/pkg/reconcilers/base_reconciler.go index 2bc0de336..6d8d683c4 100644 --- a/pkg/reconcilers/base_reconciler.go +++ b/pkg/reconcilers/base_reconciler.go @@ -37,7 +37,7 @@ import ( // MutateFn is a function which mutates the existing object into it's desired state. type MutateFn func(existing, desired client.Object) (bool, error) -func CreateOnlyMutator(existing, desired client.Object) (bool, error) { +func CreateOnlyMutator(_, _ client.Object) (bool, error) { return false, nil } diff --git a/pkg/reconcilers/base_reconciler_test.go b/pkg/reconcilers/base_reconciler_test.go index 375c60856..51d0845cc 100644 --- a/pkg/reconcilers/base_reconciler_test.go +++ b/pkg/reconcilers/base_reconciler_test.go @@ -1,5 +1,4 @@ //go:build unit -// +build unit /* Copyright 2021 Red Hat, Inc. @@ -73,11 +72,11 @@ func TestBaseReconcilerCreate(t *testing.T) { } // Objects to track in the fake client. - objs := []runtime.Object{} + var objs []runtime.Object // Create a fake client to mock API calls. - cl := fake.NewFakeClient(objs...) - clientAPIReader := fake.NewFakeClient(objs...) + cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() recorder := record.NewFakeRecorder(10000) baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) @@ -143,8 +142,8 @@ func TestBaseReconcilerUpdateNeeded(t *testing.T) { objs := []runtime.Object{existingConfigmap} // Create a fake client to mock API calls. - cl := fake.NewFakeClient(objs...) - clientAPIReader := fake.NewFakeClient(objs...) + cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() recorder := record.NewFakeRecorder(10000) baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) @@ -229,8 +228,8 @@ func TestBaseReconcilerDelete(t *testing.T) { objs := []runtime.Object{existing} // Create a fake client to mock API calls. - cl := fake.NewFakeClient(objs...) - clientAPIReader := fake.NewFakeClient(objs...) + cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() recorder := record.NewFakeRecorder(10000) baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) diff --git a/pkg/reconcilers/targetref_reconciler.go b/pkg/reconcilers/targetref_reconciler.go index a7fdcbde6..004371891 100644 --- a/pkg/reconcilers/targetref_reconciler.go +++ b/pkg/reconcilers/targetref_reconciler.go @@ -126,7 +126,7 @@ func (r *TargetRefReconciler) FetchAcceptedGatewayHTTPRoutes(ctx context.Context } // TargetedGatewayKeys returns the list of gateways that are being referenced from the target. -func (r *TargetRefReconciler) TargetedGatewayKeys(ctx context.Context, targetNetworkObject client.Object) []client.ObjectKey { +func (r *TargetRefReconciler) TargetedGatewayKeys(_ context.Context, targetNetworkObject client.Object) []client.ObjectKey { switch obj := targetNetworkObject.(type) { case *gatewayapiv1beta1.HTTPRoute: gwKeys := make([]client.ObjectKey, 0) @@ -176,7 +176,7 @@ func (r *TargetRefReconciler) ReconcileTargetBackReference(ctx context.Context, return nil } -func (r *TargetRefReconciler) DeleteTargetBackReference(ctx context.Context, policyKey client.ObjectKey, targetNetworkObject client.Object, annotationName string) error { +func (r *TargetRefReconciler) DeleteTargetBackReference(ctx context.Context, targetNetworkObject client.Object, annotationName string) error { logger, _ := logr.FromContext(ctx) targetNetworkObjectKey := client.ObjectKeyFromObject(targetNetworkObject) diff --git a/pkg/reconcilers/targetref_reconciler_test.go b/pkg/reconcilers/targetref_reconciler_test.go index 7de8e1066..46cff300e 100644 --- a/pkg/reconcilers/targetref_reconciler_test.go +++ b/pkg/reconcilers/targetref_reconciler_test.go @@ -81,8 +81,8 @@ func TestFetchValidGateway(t *testing.T) { objs := []runtime.Object{existingGateway} // Create a fake client to mock API calls. - cl := fake.NewFakeClient(objs...) - clientAPIReader := fake.NewFakeClient(objs...) + cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() recorder := record.NewFakeRecorder(1000) baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) @@ -165,8 +165,8 @@ func TestFetchValidHTTPRoute(t *testing.T) { objs := []runtime.Object{existingRoute} // Create a fake client to mock API calls. - cl := fake.NewFakeClient(objs...) - clientAPIReader := fake.NewFakeClient(objs...) + cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() recorder := record.NewFakeRecorder(1000) baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) @@ -255,8 +255,8 @@ func TestFetchValidTargetRef(t *testing.T) { objs := []runtime.Object{existingRoute} // Create a fake client to mock API calls. - cl := fake.NewFakeClient(objs...) - clientAPIReader := fake.NewFakeClient(objs...) + cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() recorder := record.NewFakeRecorder(1000) baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) @@ -285,9 +285,9 @@ func TestFetchValidTargetRef(t *testing.T) { func TestReconcileTargetBackReference(t *testing.T) { var ( - namespace = "operator-unittest" - routeName = "my-route" - annotationName string = "some-annotation" + namespace = "operator-unittest" + routeName = "my-route" + annotationName = "some-annotation" ) baseCtx := context.Background() ctx := logr.NewContext(baseCtx, log.Log) @@ -345,8 +345,8 @@ func TestReconcileTargetBackReference(t *testing.T) { objs := []runtime.Object{existingRoute} // Create a fake client to mock API calls. - cl := fake.NewFakeClient(objs...) - clientAPIReader := fake.NewFakeClient(objs...) + cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() recorder := record.NewFakeRecorder(1000) baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) @@ -442,8 +442,8 @@ func TestTargetedGatewayKeys(t *testing.T) { objs := []runtime.Object{existingRoute} // Create a fake client to mock API calls. - cl := fake.NewFakeClient(objs...) - clientAPIReader := fake.NewFakeClient(objs...) + cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() recorder := record.NewFakeRecorder(1000) baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) @@ -466,9 +466,9 @@ func TestTargetedGatewayKeys(t *testing.T) { func TestDeleteTargetBackReference(t *testing.T) { var ( - namespace = "operator-unittest" - routeName = "my-route" - annotationName string = "some-annotation" + namespace = "operator-unittest" + routeName = "my-route" + annotationName = "some-annotation" ) baseCtx := context.Background() ctx := logr.NewContext(baseCtx, log.Log) @@ -483,8 +483,6 @@ func TestDeleteTargetBackReference(t *testing.T) { t.Fatal(err) } - policyKey := client.ObjectKey{Name: "someName", Namespace: "someNamespace"} - existingRoute := &gatewayapiv1beta1.HTTPRoute{ TypeMeta: metav1.TypeMeta{ APIVersion: "gateway.networking.k8s.io/v1beta1", @@ -529,8 +527,8 @@ func TestDeleteTargetBackReference(t *testing.T) { objs := []runtime.Object{existingRoute} // Create a fake client to mock API calls. - cl := fake.NewFakeClient(objs...) - clientAPIReader := fake.NewFakeClient(objs...) + cl := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() + clientAPIReader := fake.NewClientBuilder().WithRuntimeObjects(objs...).Build() recorder := record.NewFakeRecorder(1000) baseReconciler := NewBaseReconciler(cl, s, clientAPIReader, log.Log, recorder) @@ -538,7 +536,7 @@ func TestDeleteTargetBackReference(t *testing.T) { BaseReconciler: baseReconciler, } - err = targetRefReconciler.DeleteTargetBackReference(ctx, policyKey, existingRoute, annotationName) + err = targetRefReconciler.DeleteTargetBackReference(ctx, existingRoute, annotationName) if err != nil { t.Fatal(err) } diff --git a/pkg/rlptools/rate_limit_index.go b/pkg/rlptools/rate_limit_index.go index 6c41816e4..a14d8cd93 100644 --- a/pkg/rlptools/rate_limit_index.go +++ b/pkg/rlptools/rate_limit_index.go @@ -5,7 +5,7 @@ import ( "sort" "strings" - orderedmap "github.com/elliotchance/orderedmap/v2" + "github.com/elliotchance/orderedmap/v2" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" "sigs.k8s.io/controller-runtime/pkg/client" diff --git a/pkg/rlptools/utils.go b/pkg/rlptools/utils.go index ae0163617..95d282396 100644 --- a/pkg/rlptools/utils.go +++ b/pkg/rlptools/utils.go @@ -13,11 +13,11 @@ import ( ) const ( - LimitadorRateLimitIdentitiferPrefix = "limit." + LimitadorRateLimitIdentifierPrefix = "limit." ) func LimitNameToLimitadorIdentifier(uniqueLimitName string) string { - identifier := LimitadorRateLimitIdentitiferPrefix + identifier := LimitadorRateLimitIdentifierPrefix // sanitize chars that are not allowed in limitador identifiers for _, c := range uniqueLimitName { diff --git a/pkg/rlptools/utils_test.go b/pkg/rlptools/utils_test.go index 0b85796fa..79677dba6 100644 --- a/pkg/rlptools/utils_test.go +++ b/pkg/rlptools/utils_test.go @@ -31,7 +31,7 @@ func testRLP_1Limit_1Rate(ns, name string) *kuadrantv1beta2.RateLimitPolicy { { Limit: 5, Duration: 10, - Unit: kuadrantv1beta2.TimeUnit("second"), + Unit: "second", }, }, }, @@ -57,7 +57,7 @@ func testRLP_2Limits_1Rate(ns, name string) *kuadrantv1beta2.RateLimitPolicy { { Limit: 5, Duration: 10, - Unit: kuadrantv1beta2.TimeUnit("second"), + Unit: "second", }, }, }, @@ -66,7 +66,7 @@ func testRLP_2Limits_1Rate(ns, name string) *kuadrantv1beta2.RateLimitPolicy { { Limit: 3, Duration: 1, - Unit: kuadrantv1beta2.TimeUnit("hour"), + Unit: "hour", }, }, }, @@ -92,12 +92,12 @@ func testRLP_1Limit_2Rates(ns, name string) *kuadrantv1beta2.RateLimitPolicy { { Limit: 5, Duration: 10, - Unit: kuadrantv1beta2.TimeUnit("second"), + Unit: "second", }, { Limit: 3, Duration: 1, - Unit: kuadrantv1beta2.TimeUnit("minute"), + Unit: "minute", }, }, }, @@ -120,13 +120,13 @@ func testRLP_1Limit_1Rate_1Counter(ns, name string) *kuadrantv1beta2.RateLimitPo Limits: map[string]kuadrantv1beta2.Limit{ "l1": { Counters: []kuadrantv1beta2.ContextSelector{ - kuadrantv1beta2.ContextSelector("request.path"), + "request.path", }, Rates: []kuadrantv1beta2.Rate{ { Limit: 5, Duration: 10, - Unit: kuadrantv1beta2.TimeUnit("second"), + Unit: "second", }, }, }, diff --git a/pkg/rlptools/wasm/types.go b/pkg/rlptools/wasm/types.go index 2e553ea35..98f184895 100644 --- a/pkg/rlptools/wasm/types.go +++ b/pkg/rlptools/wasm/types.go @@ -65,7 +65,7 @@ type PatternExpression struct { // Condition defines traffic matching rules type Condition struct { - // All of the expressions defined must match to match this condition + // All the expressions defined must match to match this condition // +optional AllOf []PatternExpression `json:"allOf,omitempty"` } diff --git a/pkg/rlptools/wasm_utils.go b/pkg/rlptools/wasm_utils.go index a2d9581f7..0c943700c 100644 --- a/pkg/rlptools/wasm_utils.go +++ b/pkg/rlptools/wasm_utils.go @@ -9,6 +9,7 @@ import ( _struct "google.golang.org/protobuf/types/known/structpb" istioclientgoextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" + "k8s.io/utils/env" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -18,7 +19,7 @@ import ( ) var ( - WASMFilterImageURL = common.FetchEnv("RELATED_IMAGE_WASMSHIM", "oci://quay.io/kuadrant/wasm-shim:latest") + WASMFilterImageURL = env.GetString("RELATED_IMAGE_WASMSHIM", "oci://quay.io/kuadrant/wasm-shim:latest") ) // WasmRules computes WASM rules from the policy and the targeted route. diff --git a/pkg/rlptools/wasm_utils_test.go b/pkg/rlptools/wasm_utils_test.go index 9771679d3..89e6a5fbe 100644 --- a/pkg/rlptools/wasm_utils_test.go +++ b/pkg/rlptools/wasm_utils_test.go @@ -30,7 +30,7 @@ func TestWasmRules(t *testing.T) { Type: &[]gatewayapiv1beta1.PathMatchType{gatewayapiv1beta1.PathMatchPathPrefix}[0], Value: &[]string{"/toy"}[0], }, - Method: &[]gatewayapiv1beta1.HTTPMethod{gatewayapiv1beta1.HTTPMethod("GET")}[0], + Method: &[]gatewayapiv1beta1.HTTPMethod{"GET"}[0], }, }, }, @@ -169,7 +169,7 @@ func TestWasmRules(t *testing.T) { Type: &[]gatewayapiv1beta1.PathMatchType{gatewayapiv1beta1.PathMatchPathPrefix}[0], Value: &[]string{"/toy"}[0], }, - Method: &[]gatewayapiv1beta1.HTTPMethod{gatewayapiv1beta1.HTTPMethod("GET")}[0], + Method: &[]gatewayapiv1beta1.HTTPMethod{"GET"}[0], }, }, }, @@ -264,7 +264,7 @@ func TestWasmRules(t *testing.T) { { Matches: []gatewayapiv1beta1.HTTPRouteMatch{ { - Method: &[]gatewayapiv1beta1.HTTPMethod{gatewayapiv1beta1.HTTPMethod("POST")}[0], + Method: &[]gatewayapiv1beta1.HTTPMethod{"POST"}[0], }, }, },