From 56334b10f230f90939a82ad4dbd9d97b6c1e7abb Mon Sep 17 00:00:00 2001 From: Tharsanan1 Date: Mon, 9 Sep 2024 09:22:39 +0530 Subject: [PATCH] Fix revive and test failes --- .../oasparser/envoyconf/routes_configs.go | 17 ++-- .../oasparser/model/adapter_internal_api.go | 8 +- .../operator/controllers/dp/api_controller.go | 36 ++++----- adapter/internal/operator/utils/utils.go | 6 +- common-controller/build.gradle | 4 +- common-go-libs/build.gradle | 4 +- common-go-libs/controllers/dp/suite_test.go | 81 ------------------- 7 files changed, 39 insertions(+), 117 deletions(-) delete mode 100644 common-go-libs/controllers/dp/suite_test.go diff --git a/adapter/internal/oasparser/envoyconf/routes_configs.go b/adapter/internal/oasparser/envoyconf/routes_configs.go index 2ce5e3212..197791dea 100644 --- a/adapter/internal/oasparser/envoyconf/routes_configs.go +++ b/adapter/internal/oasparser/envoyconf/routes_configs.go @@ -43,6 +43,10 @@ import ( v35 "github.com/envoyproxy/go-control-plane/envoy/type/metadata/v3" ) +const( + authzNamespace = "envoy.filters.http.ext_authz" +) + func generateRouteConfig(routeName string, match *routev3.RouteMatch, action *routev3.Route_Route, redirectAction *routev3.Route_Redirect, metadata *corev3.Metadata, decorator *routev3.Decorator, typedPerFilterConfig map[string]*anypb.Any, requestHeadersToAdd []*corev3.HeaderValueOption, requestHeadersToRemove []string, @@ -172,9 +176,13 @@ func mapStatusCodeToEnum(statusCode int) int { } } const ( + // DescriptorKeyForAIRequestTokenCount is the descriptor key for AI request token count ratelimit DescriptorKeyForAIRequestTokenCount = "airequesttokencount" + // DescriptorKeyForAIResponseTokenCount is the descriptor key for AI response token count ratelimit DescriptorKeyForAIResponseTokenCount = "airesponsetokencount" + // DescriptorKeyForAITotalTokenCount is the descriptor key for AI total token count ratelimit DescriptorKeyForAITotalTokenCount = "aitotaltokencount" + // DescriptorKeyForAIRequestCount is the descriptor key for AI request count ratelimit DescriptorKeyForAIRequestCount = "airequestcount" ) @@ -230,9 +238,6 @@ func generateBackendBasedAIRatelimit(descValue string) []*routev3.RateLimit { return []*routev3.RateLimit{&rateLimitForRequestTokenCount, &rateLimitForResponseTokenCount, &rateLimitForRequestCount, &rateLimitForTotalTokenCount} } -const( - authz_namespace = "envoy.filters.http.ext_authz" -) func generateSubscriptionBasedAIRatelimit(descValue string) []*routev3.RateLimit { rateLimitForRequestTokenCount := routev3.RateLimit{ @@ -242,7 +247,7 @@ func generateSubscriptionBasedAIRatelimit(descValue string) []*routev3.RateLimit Metadata: &routev3.RateLimit_Action_MetaData{ DescriptorKey: DescriptorKeyForAIRequestTokenCount, MetadataKey: &v35.MetadataKey{ - Key: authz_namespace, + Key: authzNamespace, Path: []*v35.MetadataKey_PathSegment{ &v35.MetadataKey_PathSegment{ Segment: &v35.MetadataKey_PathSegment_Key{ @@ -263,7 +268,7 @@ func generateSubscriptionBasedAIRatelimit(descValue string) []*routev3.RateLimit Metadata: &routev3.RateLimit_Action_MetaData{ DescriptorKey: DescriptorKeyForAIResponseTokenCount, MetadataKey: &v35.MetadataKey{ - Key: authz_namespace, + Key: authzNamespace, Path: []*v35.MetadataKey_PathSegment{ &v35.MetadataKey_PathSegment{ Segment: &v35.MetadataKey_PathSegment_Key{ @@ -284,7 +289,7 @@ func generateSubscriptionBasedAIRatelimit(descValue string) []*routev3.RateLimit Metadata: &routev3.RateLimit_Action_MetaData{ DescriptorKey: DescriptorKeyForAIRequestCount, MetadataKey: &v35.MetadataKey{ - Key: authz_namespace, + Key: authzNamespace, Path: []*v35.MetadataKey_PathSegment{ &v35.MetadataKey_PathSegment{ Segment: &v35.MetadataKey_PathSegment_Key{ diff --git a/adapter/internal/oasparser/model/adapter_internal_api.go b/adapter/internal/oasparser/model/adapter_internal_api.go index b7a5c2d5e..c9d84609e 100644 --- a/adapter/internal/oasparser/model/adapter_internal_api.go +++ b/adapter/internal/oasparser/model/adapter_internal_api.go @@ -469,7 +469,7 @@ func (adapterInternalAPI *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwap ratelimitPolicy = *outputRatelimitPolicy } - for ruleId, rule := range httpRoute.Spec.Rules { + for ruleID, rule := range httpRoute.Spec.Rules { var endPoints []Endpoint var policies = OperationPolicies{} var circuitBreaker *dpv1alpha1.CircuitBreaker @@ -493,12 +493,12 @@ func (adapterInternalAPI *AdapterInternalAPI) SetInfoHTTPRouteCR(httpRoute *gwap enableBackendBasedAIRatelimit := false descriptorValue := "" - if aiRatelimitPolicy, exists := ruleIdxToAiRatelimitPolicyMapping[ruleId]; exists { - loggers.LoggerAPI.Infof("Found AI ratelimit mapping for ruleId: %d, related api: %s", ruleId, adapterInternalAPI.UUID) + if aiRatelimitPolicy, exists := ruleIdxToAiRatelimitPolicyMapping[ruleID]; exists { + loggers.LoggerAPI.Infof("Found AI ratelimit mapping for ruleId: %d, related api: %s", ruleID, adapterInternalAPI.UUID) enableBackendBasedAIRatelimit = true descriptorValue = prepareAIRatelimitIdentifier(adapterInternalAPI.OrganizationID, utils.NamespacedName(aiRatelimitPolicy), &aiRatelimitPolicy.Spec) } else { - loggers.LoggerAPI.Infof("Could not find AIratelimit for ruleId: %d, len of map: %d, related api: %s", ruleId, len(ruleIdxToAiRatelimitPolicyMapping), adapterInternalAPI.UUID) + loggers.LoggerAPI.Infof("Could not find AIratelimit for ruleId: %d, len of map: %d, related api: %s", ruleID, len(ruleIdxToAiRatelimitPolicyMapping), adapterInternalAPI.UUID) } backendBasePath := "" diff --git a/adapter/internal/operator/controllers/dp/api_controller.go b/adapter/internal/operator/controllers/dp/api_controller.go index 16e2acc42..0265c8b59 100644 --- a/adapter/internal/operator/controllers/dp/api_controller.go +++ b/adapter/internal/operator/controllers/dp/api_controller.go @@ -99,7 +99,7 @@ const ( aiRatelimitPolicyToBackendIndex = "aiRatelimitPolicyToBackendIndex" aiRatelimitPolicyToSubscriptionIndex = "aiRatelimitPolicyToSubscriptionIndex" subscriptionToAPIIndex = "subscriptionToAPIIndex" - ApiToSubscriptionIndex = "ApiToSubscriptionIndex" + apiToSubscriptionIndex = "apiToSubscriptionIndex" ) var ( @@ -838,22 +838,21 @@ func (apiReconciler *APIReconciler) resolveAiSubscriptionRatelimitPolicies(ctx c apiState.IsAiSubscriptionRatelimitEnabled = false subscriptionList := &cpv1alpha2.SubscriptionList{} if err := apiReconciler.client.List(ctx, subscriptionList, &k8client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector(subscriptionToAPIIndex, utils.GetSubscriptionToAPIIndexId(apiState.APIDefinition.Spec.APIName, apiState.APIDefinition.Spec.APIVersion)), + FieldSelector: fields.OneTermEqualSelector(subscriptionToAPIIndex, utils.GetSubscriptionToAPIIndexID(apiState.APIDefinition.Spec.APIName, apiState.APIDefinition.Spec.APIVersion)), }); err != nil { loggers.LoggerAPKOperator.Infof("No associated subscription found for API: %s", utils.NamespacedName(apiState.APIDefinition)) return - } else { - for _, subscription := range subscriptionList.Items { - aiRatelimitPolicyList := &dpv1alpha3.AIRateLimitPolicyList{} - if err := apiReconciler.client.List(ctx, aiRatelimitPolicyList, &k8client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector(aiRatelimitPolicyToSubscriptionIndex, utils.NamespacedName(&subscription).String()), - }); err != nil { - loggers.LoggerAPKOperator.Infof("No associated aiRatelimitPolicy found for Subscription: %s", utils.NamespacedName(&subscription)) - return - } - if len(aiRatelimitPolicyList.Items) > 0 { - apiState.IsAiSubscriptionRatelimitEnabled = true - } + } + for _, subscription := range subscriptionList.Items { + aiRatelimitPolicyList := &dpv1alpha3.AIRateLimitPolicyList{} + if err := apiReconciler.client.List(ctx, aiRatelimitPolicyList, &k8client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector(aiRatelimitPolicyToSubscriptionIndex, utils.NamespacedName(&subscription).String()), + }); err != nil { + loggers.LoggerAPKOperator.Infof("No associated aiRatelimitPolicy found for Subscription: %s", utils.NamespacedName(&subscription)) + return + } + if len(aiRatelimitPolicyList.Items) > 0 { + apiState.IsAiSubscriptionRatelimitEnabled = true } } } @@ -924,7 +923,7 @@ func (apiReconciler *APIReconciler) getResolvedBackendsMapping(ctx context.Conte return nil, fmt.Errorf("unable to find backend %s", backendNamespacedName.String()) } } - + } for _, filter := range rule.Filters { @@ -1456,9 +1455,8 @@ func (apiReconciler *APIReconciler) getAPIsForAIRatelimitPolicy(ctx context.Cont if err := apiReconciler.client.Get(ctx, namespacedName, backend); err != nil { loggers.LoggerAPKOperator.ErrorC(logging.PrintError(logging.Error2621, logging.MINOR, "Unable to find associated Backend for AIratelimitPolicy targetref: %s", namespacedName.String())) return []reconcile.Request{} - } else { - return apiReconciler.getAPIsForBackend(ctx, backend) } + return apiReconciler.getAPIsForBackend(ctx, backend) } return []reconcile.Request{} } @@ -1473,7 +1471,7 @@ func (apiReconciler *APIReconciler) getAPIsForSubscription(ctx context.Context, } apiList := &dpv1alpha2.APIList{} if err := apiReconciler.client.List(ctx, apiList, &k8client.ListOptions{ - FieldSelector: fields.OneTermEqualSelector(ApiToSubscriptionIndex, utils.GetSubscriptionToAPIIndexId(subscription.Spec.API.Name, subscription.Spec.API.Version)), + FieldSelector: fields.OneTermEqualSelector(apiToSubscriptionIndex, utils.GetSubscriptionToAPIIndexID(subscription.Spec.API.Name, subscription.Spec.API.Version)), }); err != nil { loggers.LoggerAPKOperator.ErrorC(logging.PrintError(logging.Error2649, logging.CRITICAL, "Unable to find associated APIs for subscription: %s, error: %v", utils.NamespacedName(subscription).String(), err.Error())) return []reconcile.Request{} @@ -2058,7 +2056,7 @@ func addIndexes(ctx context.Context, mgr manager.Manager) error { } // API to Subscription indexer - if err := mgr.GetFieldIndexer().IndexField(ctx, &dpv1alpha2.API{}, ApiToSubscriptionIndex, + if err := mgr.GetFieldIndexer().IndexField(ctx, &dpv1alpha2.API{}, apiToSubscriptionIndex, func(rawObj k8client.Object) []string { api := rawObj.(*dpv1alpha2.API) var apis []string diff --git a/adapter/internal/operator/utils/utils.go b/adapter/internal/operator/utils/utils.go index 76c57934b..081f71f76 100644 --- a/adapter/internal/operator/utils/utils.go +++ b/adapter/internal/operator/utils/utils.go @@ -647,7 +647,7 @@ func ContainsString(list []string, target string) bool { return false } -// GetSubscriptionToAPIIndexId returns the id which can be used to list subscriptions related to a api. -func GetSubscriptionToAPIIndexId(name string, version string) string { +// GetSubscriptionToAPIIndexID returns the id which can be used to list subscriptions related to a api. +func GetSubscriptionToAPIIndexID(name string, version string) string { return fmt.Sprintf("%s_%s", name, version) -} \ No newline at end of file +} diff --git a/common-controller/build.gradle b/common-controller/build.gradle index 0050837e7..7982534fc 100644 --- a/common-controller/build.gradle +++ b/common-controller/build.gradle @@ -69,8 +69,8 @@ tasks.named('go_revive_run').configure { } tasks.named('go_build').configure { - // dependsOn go_revive_run - // dependsOn go_vet + dependsOn go_revive_run + dependsOn go_vet println("Running go build") finalizedBy docker_build } diff --git a/common-go-libs/build.gradle b/common-go-libs/build.gradle index 18d1622c9..13dd90184 100644 --- a/common-go-libs/build.gradle +++ b/common-go-libs/build.gradle @@ -46,8 +46,8 @@ tasks.named('go_revive_run').configure { } tasks.named('go_build').configure { - // dependsOn go_revive_run - // dependsOn go_vet + dependsOn go_revive_run + dependsOn go_vet println("Running go build") } diff --git a/common-go-libs/controllers/dp/suite_test.go b/common-go-libs/controllers/dp/suite_test.go deleted file mode 100644 index 5fbd887c6..000000000 --- a/common-go-libs/controllers/dp/suite_test.go +++ /dev/null @@ -1,81 +0,0 @@ -/* - * Copyright (c) 2023, WSO2 LLC. (http://www.wso2.org) All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -package dp - -import ( - "path/filepath" - "testing" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/envtest" - logf "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/log/zap" - - dpv1alpha3 "github.com/wso2/apk/common-go-libs/apis/dp/v1alpha3" - //+kubebuilder:scaffold:imports -) - -// These tests use Ginkgo (BDD-style Go testing framework). Refer to -// http://onsi.github.io/ginkgo/ to learn more about Ginkgo. - -var cfg *rest.Config -var k8sClient client.Client -var testEnv *envtest.Environment - -func TestAPIs(t *testing.T) { - RegisterFailHandler(Fail) - - RunSpecs(t, "Controller Suite") -} - -var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) - - By("bootstrapping test environment") - testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, - ErrorIfCRDPathMissing: true, - } - - var err error - // cfg is defined in this file globally. - cfg, err = testEnv.Start() - Expect(err).NotTo(HaveOccurred()) - Expect(cfg).NotTo(BeNil()) - - err = dpv1alpha3.AddToScheme(scheme.Scheme) - Expect(err).NotTo(HaveOccurred()) - - //+kubebuilder:scaffold:scheme - - k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) - Expect(err).NotTo(HaveOccurred()) - Expect(k8sClient).NotTo(BeNil()) - -}) - -var _ = AfterSuite(func() { - By("tearing down the test environment") - err := testEnv.Stop() - Expect(err).NotTo(HaveOccurred()) -})