diff --git a/docs/guides/service-name-override.md b/docs/guides/service-name-override.md new file mode 100644 index 00000000..e1937c2a --- /dev/null +++ b/docs/guides/service-name-override.md @@ -0,0 +1,41 @@ +# Service Name Override + +Override default generated VPC Lattice service names for Routes to avoid conflicts or implement naming conventions. + +## Usage + +Add the annotation to HTTPRoute or GRPCRoute: + +```yaml +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: api-service + namespace: production + annotations: + application-networking.k8s.aws/service-name-override: "custom-service-name" +spec: + parentRefs: + - name: my-gateway + rules: + - backendRefs: + - name: backend-service + port: 80 +``` + +## Before You Start + +Service name overrides must be set during route creation and cannot be modified. Adding or changing the annotation on existing routes does not work. + +**To add or change a service name override for existing route:** +1. Delete the route: `kubectl delete httproute my-route` +2. Update annotation and recreate: `kubectl apply -f updated-route.yaml` + +## Validation Rules + +Service names must: +- Be 3-40 characters long +- Use only lowercase letters, numbers, and hyphens +- Start and end with alphanumeric characters +- Not start with `svc-` +- Not contain `--` or start/end with `-` diff --git a/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go b/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go index 4c599ff8..4a479e20 100644 --- a/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go +++ b/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go @@ -10,7 +10,8 @@ import ( ) const ( - AccessLogSubscriptionAnnotationKey = k8s.AnnotationPrefix + "accessLogSubscription" + AccessLogSubscriptionAnnotationKey = k8s.AnnotationPrefix + "accessLogSubscription" + AccessLogSubscriptionResourceNameAnnotationKey = k8s.AnnotationPrefix + "accessLogSubscriptionResourceName" ) // +genclient diff --git a/pkg/controllers/accesslogpolicy_controller.go b/pkg/controllers/accesslogpolicy_controller.go index 0dc650c7..ac3f7655 100644 --- a/pkg/controllers/accesslogpolicy_controller.go +++ b/pkg/controllers/accesslogpolicy_controller.go @@ -134,15 +134,25 @@ func (r *accessLogPolicyReconciler) reconcile(ctx context.Context, req ctrl.Requ r.eventRecorder.Event(alp, corev1.EventTypeNormal, k8s.ReconcilingEvent, "Started reconciling") + var err error if !alp.DeletionTimestamp.IsZero() { - return r.reconcileDelete(ctx, alp) + err = r.reconcileDelete(ctx, alp) } else { - return r.reconcileUpsert(ctx, alp) + err = r.reconcileUpsert(ctx, alp) } + + if err != nil { + r.eventRecorder.Event(alp, corev1.EventTypeWarning, k8s.FailedReconcileEvent, fmt.Sprintf("Reconcile failed: %s", err)) + return err + } + + r.eventRecorder.Event(alp, corev1.EventTypeNormal, k8s.ReconciledEvent, "Successfully reconciled") + return nil } func (r *accessLogPolicyReconciler) reconcileDelete(ctx context.Context, alp *anv1alpha1.AccessLogPolicy) error { - _, err := r.buildAndDeployModel(ctx, alp) + targetRefName := alp.Annotations[anv1alpha1.AccessLogSubscriptionResourceNameAnnotationKey] + _, err := r.buildAndDeployModel(ctx, alp, targetRefName) if err != nil { r.eventRecorder.Event(alp, corev1.EventTypeWarning, k8s.FailedReconcileEvent, fmt.Sprintf("Failed to delete due to %s", err)) @@ -189,7 +199,7 @@ func (r *accessLogPolicyReconciler) reconcileUpsert(ctx context.Context, alp *an return r.updateAccessLogPolicyStatus(ctx, alp, gwv1alpha2.PolicyReasonInvalid, message) } - targetRefExists, err := r.targetRefExists(ctx, alp) + targetRefExists, targetRef, err := r.targetRefExists(ctx, alp) if err != nil { return err } @@ -199,7 +209,15 @@ func (r *accessLogPolicyReconciler) reconcileUpsert(ctx context.Context, alp *an return r.updateAccessLogPolicyStatus(ctx, alp, gwv1alpha2.PolicyReasonTargetNotFound, message) } - stack, err := r.buildAndDeployModel(ctx, alp) + targetRefName, err := r.targetRefToResourceName(alp, targetRef) + if err != nil { + if k8s.IsInvalidServiceNameOverrideError(err) { + return r.updateAccessLogPolicyStatus(ctx, alp, gwv1alpha2.PolicyReasonInvalid, err.Error()) + } + return err + } + + stack, err := r.buildAndDeployModel(ctx, alp, targetRefName) if err != nil { if services.IsConflictError(err) { message := "An Access Log Policy with a Destination Arn for the same destination type already exists for this targetRef" @@ -215,7 +233,7 @@ func (r *accessLogPolicyReconciler) reconcileUpsert(ctx context.Context, alp *an return err } - err = r.updateAccessLogPolicyAnnotations(ctx, alp, stack) + err = r.updateAccessLogPolicyAnnotations(ctx, alp, stack, targetRefName) if err != nil { return err } @@ -230,40 +248,46 @@ func (r *accessLogPolicyReconciler) reconcileUpsert(ctx context.Context, alp *an return nil } -func (r *accessLogPolicyReconciler) targetRefExists(ctx context.Context, alp *anv1alpha1.AccessLogPolicy) (bool, error) { +func (r *accessLogPolicyReconciler) targetRefExists(ctx context.Context, alp *anv1alpha1.AccessLogPolicy) (bool, metav1.Object, error) { targetRefNamespacedName := types.NamespacedName{ Name: string(alp.Spec.TargetRef.Name), Namespace: k8s.NamespaceOrDefault(alp.Spec.TargetRef.Namespace), } var err error + var targetObj metav1.Object switch alp.Spec.TargetRef.Kind { case "Gateway": gw := &gwv1.Gateway{} err = r.client.Get(ctx, targetRefNamespacedName, gw) + targetObj = gw case "HTTPRoute": - httpRoute := &gwv1.HTTPRoute{} - err = r.client.Get(ctx, targetRefNamespacedName, httpRoute) + route := &gwv1.HTTPRoute{} + err = r.client.Get(ctx, targetRefNamespacedName, route) + targetObj = route case "GRPCRoute": - grpcRoute := &gwv1.GRPCRoute{} - err = r.client.Get(ctx, targetRefNamespacedName, grpcRoute) + route := &gwv1.GRPCRoute{} + err = r.client.Get(ctx, targetRefNamespacedName, route) + targetObj = route default: - return false, fmt.Errorf("access Log Policy targetRef is for unsupported Kind: %s", alp.Spec.TargetRef.Kind) + return false, nil, fmt.Errorf("access Log Policy targetRef is for unsupported Kind: %s", alp.Spec.TargetRef.Kind) } if err != nil && !apierrors.IsNotFound(err) { - return false, err + return false, nil, err } - return err == nil, nil + exists := err == nil + return exists, targetObj, nil } func (r *accessLogPolicyReconciler) buildAndDeployModel( ctx context.Context, alp *anv1alpha1.AccessLogPolicy, + targetRefName string, ) (core.Stack, error) { - stack, _, err := r.modelBuilder.Build(ctx, alp) + stack, _, err := r.modelBuilder.Build(ctx, alp, targetRefName) if err != nil { return nil, err } @@ -286,6 +310,7 @@ func (r *accessLogPolicyReconciler) updateAccessLogPolicyAnnotations( ctx context.Context, alp *anv1alpha1.AccessLogPolicy, stack core.Stack, + targetRefName string, ) error { var accessLogSubscriptions []*model.AccessLogSubscription err := stack.ListResources(&accessLogSubscriptions) @@ -300,6 +325,7 @@ func (r *accessLogPolicyReconciler) updateAccessLogPolicyAnnotations( alp.Annotations = make(map[string]string) } alp.Annotations[anv1alpha1.AccessLogSubscriptionAnnotationKey] = als.Status.Arn + alp.Annotations[anv1alpha1.AccessLogSubscriptionResourceNameAnnotationKey] = targetRefName if err := r.client.Patch(ctx, alp, client.MergeFrom(oldAlp)); err != nil { r.eventRecorder.Event(alp, corev1.EventTypeWarning, k8s.FailedReconcileEvent, "Failed to update annotation due to "+err.Error()) @@ -375,3 +401,30 @@ func (r *accessLogPolicyReconciler) findImpactedAccessLogPolicies(ctx context.Co return requests } + +func (r *accessLogPolicyReconciler) targetRefToResourceName( + alp *anv1alpha1.AccessLogPolicy, + targetObj metav1.Object, +) (string, error) { + targetRef := alp.Spec.TargetRef + + if targetRef.Kind == "Gateway" { + return string(targetRef.Name), nil + } + + if targetRef.Kind == "HTTPRoute" || targetRef.Kind == "GRPCRoute" { + namespace := alp.Namespace + if targetRef.Namespace != nil { + namespace = string(*targetRef.Namespace) + } + + serviceNameOverride, err := k8s.GetServiceNameOverrideWithValidation(targetObj) + if err != nil { + return "", fmt.Errorf("route '%s' has %w", targetRef.Name, err) + } + + return utils.LatticeServiceName(string(targetRef.Name), namespace, serviceNameOverride), nil + } + + return "", fmt.Errorf("unsupported targetRef kind: %s", targetRef.Kind) +} diff --git a/pkg/controllers/accesslogpolicy_controller_test.go b/pkg/controllers/accesslogpolicy_controller_test.go new file mode 100644 index 00000000..e16f04ad --- /dev/null +++ b/pkg/controllers/accesslogpolicy_controller_test.go @@ -0,0 +1,245 @@ +package controllers + +import ( + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gwv1 "sigs.k8s.io/gateway-api/apis/v1" + gwv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1" + "github.com/aws/aws-application-networking-k8s/pkg/k8s" +) + +func TestAccessLogPolicyReconciler_targetRefToResourceName(t *testing.T) { + r := &accessLogPolicyReconciler{} + + tests := []struct { + name string + alp *anv1alpha1.AccessLogPolicy + targetObj metav1.Object + expected string + expectError bool + errorContains string + }{ + { + name: "Gateway targetRef returns gateway name", + alp: &anv1alpha1.AccessLogPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ + Kind: "Gateway", + Name: "test-gateway", + }, + }, + }, + targetObj: nil, + expected: "test-gateway", + expectError: false, + }, + { + name: "HTTPRoute targetRef without namespace uses policy namespace", + alp: &anv1alpha1.AccessLogPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "policy-namespace", + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ + Kind: "HTTPRoute", + Name: "test-route", + }, + }, + }, + targetObj: &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "policy-namespace", + }, + }, + expected: "test-route-policy-namespace", + expectError: false, + }, + { + name: "HTTPRoute targetRef with namespace uses specified namespace", + alp: &anv1alpha1.AccessLogPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "policy-namespace", + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ + Kind: "HTTPRoute", + Name: "test-route", + Namespace: (*gwv1alpha2.Namespace)(&[]string{"route-namespace"}[0]), + }, + }, + }, + targetObj: &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "route-namespace", + }, + }, + expected: "test-route-route-namespace", + expectError: false, + }, + { + name: "HTTPRoute targetRef with valid service name override", + alp: &anv1alpha1.AccessLogPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ + Kind: "HTTPRoute", + Name: "test-route", + }, + }, + }, + targetObj: &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + Annotations: map[string]string{ + k8s.ServiceNameOverrideAnnotation: "custom-service-name", + }, + }, + }, + expected: "custom-service-name", + expectError: false, + }, + { + name: "HTTPRoute targetRef with invalid service name override", + alp: &anv1alpha1.AccessLogPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ + Kind: "HTTPRoute", + Name: "test-route", + }, + }, + }, + targetObj: &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "test-namespace", + Annotations: map[string]string{ + k8s.ServiceNameOverrideAnnotation: "svc-invalid-name", + }, + }, + }, + expected: "", + expectError: true, + errorContains: "invalid service name override", + }, + { + name: "GRPCRoute targetRef without namespace uses policy namespace", + alp: &anv1alpha1.AccessLogPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "policy-namespace", + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ + Kind: "GRPCRoute", + Name: "test-grpc-route", + }, + }, + }, + targetObj: &gwv1.GRPCRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-grpc-route", + Namespace: "policy-namespace", + }, + }, + expected: "test-grpc-route-policy-namespace", + expectError: false, + }, + { + name: "GRPCRoute targetRef with namespace uses specified namespace", + alp: &anv1alpha1.AccessLogPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "policy-namespace", + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ + Kind: "GRPCRoute", + Name: "test-grpc-route", + Namespace: (*gwv1alpha2.Namespace)(&[]string{"route-namespace"}[0]), + }, + }, + }, + targetObj: &gwv1.GRPCRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-grpc-route", + Namespace: "route-namespace", + }, + }, + expected: "test-grpc-route-route-namespace", + expectError: false, + }, + { + name: "GRPCRoute targetRef with valid service name override", + alp: &anv1alpha1.AccessLogPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ + Kind: "GRPCRoute", + Name: "test-grpc-route", + }, + }, + }, + targetObj: &gwv1.GRPCRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-grpc-route", + Namespace: "test-namespace", + Annotations: map[string]string{ + k8s.ServiceNameOverrideAnnotation: "custom-grpc-service", + }, + }, + }, + expected: "custom-grpc-service", + expectError: false, + }, + { + name: "unsupported targetRef kind returns error", + alp: &anv1alpha1.AccessLogPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ + Kind: "Service", + Name: "test-service", + }, + }, + }, + targetObj: nil, + expected: "", + expectError: true, + errorContains: "unsupported targetRef kind", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := r.targetRefToResourceName(tt.alp, tt.targetObj) + + if tt.expectError { + assert.Error(t, err, "Expected error for test case: %s", tt.name) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains, "Error message should contain expected text") + } + assert.Equal(t, tt.expected, result, "Result should match expected value even on error") + } else { + assert.NoError(t, err, "Expected no error for test case: %s", tt.name) + assert.Equal(t, tt.expected, result, "Result should match expected value") + } + }) + } +} diff --git a/pkg/controllers/iamauthpolicy_controller.go b/pkg/controllers/iamauthpolicy_controller.go index bb2d9b10..a94ea411 100644 --- a/pkg/controllers/iamauthpolicy_controller.go +++ b/pkg/controllers/iamauthpolicy_controller.go @@ -2,6 +2,7 @@ package controllers import ( "context" + "fmt" anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1" pkg_aws "github.com/aws/aws-application-networking-k8s/pkg/aws" @@ -10,7 +11,10 @@ import ( "github.com/aws/aws-application-networking-k8s/pkg/k8s" policy "github.com/aws/aws-application-networking-k8s/pkg/k8s/policyhelper" model "github.com/aws/aws-application-networking-k8s/pkg/model/lattice" + "github.com/aws/aws-application-networking-k8s/pkg/utils" "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -19,13 +23,15 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" gwv1 "sigs.k8s.io/gateway-api/apis/v1" + gwv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) const ( - IAMAuthPolicyAnnotation = "iam-auth-policy" - IAMAuthPolicyAnnotationResId = k8s.AnnotationPrefix + IAMAuthPolicyAnnotation + "-resource-id" - IAMAuthPolicyAnnotationType = k8s.AnnotationPrefix + IAMAuthPolicyAnnotation + "-resource-type" - IAMAuthPolicyFinalizer = k8s.AnnotationPrefix + IAMAuthPolicyAnnotation + IAMAuthPolicyAnnotation = "iam-auth-policy" + IAMAuthPolicyAnnotationResId = k8s.AnnotationPrefix + IAMAuthPolicyAnnotation + "-resource-id" + IAMAuthPolicyAnnotationType = k8s.AnnotationPrefix + IAMAuthPolicyAnnotation + "-resource-type" + IAMAuthPolicyFinalizer = k8s.AnnotationPrefix + IAMAuthPolicyAnnotation + IAMAuthPolicyAnnotationResName = k8s.AnnotationPrefix + IAMAuthPolicyAnnotation + "-resource-name" ) type ( @@ -33,22 +39,24 @@ type ( ) type IAMAuthPolicyController struct { - log gwlog.Logger - client client.Client - pm *deploy.IAMAuthPolicyManager - ph *policy.PolicyHandler[*IAP] - cloud pkg_aws.Cloud + log gwlog.Logger + client client.Client + pm *deploy.IAMAuthPolicyManager + ph *policy.PolicyHandler[*IAP] + cloud pkg_aws.Cloud + eventRecorder record.EventRecorder } func RegisterIAMAuthPolicyController(log gwlog.Logger, mgr ctrl.Manager, cloud pkg_aws.Cloud) error { ph := policy.NewIAMAuthPolicyHandler(log, mgr.GetClient()) controller := &IAMAuthPolicyController{ - log: log, - client: mgr.GetClient(), - pm: deploy.NewIAMAuthPolicyManager(cloud), - ph: ph, - cloud: cloud, + log: log, + client: mgr.GetClient(), + pm: deploy.NewIAMAuthPolicyManager(cloud), + ph: ph, + cloud: cloud, + eventRecorder: mgr.GetEventRecorderFor("iam-auth-policy-controller"), } b := ctrl. @@ -88,6 +96,8 @@ func (c *IAMAuthPolicyController) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, client.IgnoreNotFound(err) } + c.eventRecorder.Event(k8sPolicy, corev1.EventTypeNormal, k8s.ReconcilingEvent, "Started reconciling") + c.log.Infow(ctx, "reconcile IAM policy", "req", req, "targetRef", k8sPolicy.Spec.TargetRef) isDelete := !k8sPolicy.DeletionTimestamp.IsZero() @@ -98,6 +108,7 @@ func (c *IAMAuthPolicyController) Reconcile(ctx context.Context, req ctrl.Reques res, err = c.reconcileUpsert(ctx, k8sPolicy) } if err != nil { + c.eventRecorder.Event(k8sPolicy, corev1.EventTypeWarning, k8s.FailedReconcileEvent, fmt.Sprintf("Reconcile failed: %s", err)) return ctrl.Result{}, err } @@ -106,6 +117,8 @@ func (c *IAMAuthPolicyController) Reconcile(ctx context.Context, req ctrl.Reques return reconcile.Result{}, err } + c.eventRecorder.Event(k8sPolicy, corev1.EventTypeNormal, k8s.ReconciledEvent, "Successfully reconciled") + c.log.Infow(ctx, "reconciled IAM policy", "req", req, "targetRef", k8sPolicy.Spec.TargetRef, @@ -117,7 +130,9 @@ func (c *IAMAuthPolicyController) Reconcile(ctx context.Context, req ctrl.Reques func (c *IAMAuthPolicyController) reconcileDelete(ctx context.Context, k8sPolicy *anv1alpha1.IAMAuthPolicy) (ctrl.Result, error) { err := c.ph.ValidateTargetRef(ctx, k8sPolicy) if err == nil { - modelPolicy := model.NewIAMAuthPolicy(k8sPolicy) + existingModel, _ := c.getLatticeAnnotation(k8sPolicy) + modelPolicy := model.NewIAMAuthPolicy(k8sPolicy, existingModel.Name) + _, err := c.pm.Delete(ctx, modelPolicy) if err != nil { return ctrl.Result{}, services.IgnoreNotFound(err) @@ -139,7 +154,19 @@ func (c *IAMAuthPolicyController) reconcileUpsert(ctx context.Context, k8sPolicy if reason != policy.ReasonAccepted { return ctrl.Result{}, nil } - modelPolicy := model.NewIAMAuthPolicy(k8sPolicy) + + resourceName, err := c.targetRefToResourceName(ctx, k8sPolicy) + if err != nil { + if k8s.IsInvalidServiceNameOverrideError(err) { + if statusErr := c.ph.UpdateAcceptedCondition(ctx, k8sPolicy, gwv1alpha2.PolicyReasonInvalid, err.Error()); statusErr != nil { + return ctrl.Result{}, statusErr + } + } + return ctrl.Result{}, err + } + + modelPolicy := model.NewIAMAuthPolicy(k8sPolicy, resourceName) + c.addFinalizer(k8sPolicy) err = c.client.Update(ctx, k8sPolicy) if err != nil { @@ -149,11 +176,11 @@ func (c *IAMAuthPolicyController) reconcileUpsert(ctx context.Context, k8sPolicy if err != nil { return reconcile.Result{}, services.IgnoreNotFound(err) } - c.updateLatticeAnnotaion(k8sPolicy, statusPolicy.ResourceId, modelPolicy.Type) err = c.handleLatticeResourceChange(ctx, k8sPolicy, statusPolicy) if err != nil { - return reconcile.Result{}, err + return ctrl.Result{}, err } + c.updateLatticeAnnotaion(k8sPolicy, statusPolicy.ResourceId, modelPolicy.Type, modelPolicy.Name) return ctrl.Result{}, nil } @@ -170,6 +197,7 @@ func (c *IAMAuthPolicyController) addFinalizer(k8sPolicy *anv1alpha1.IAMAuthPoli } // cleanup lattice resources after targetRef changes +// compares old ResourceId from k8s annotation vs new ResourceId from model func (c *IAMAuthPolicyController) handleLatticeResourceChange(ctx context.Context, k8sPolicy *anv1alpha1.IAMAuthPolicy, statusPolicy model.IAMAuthPolicyStatus) error { prevModel, ok := c.getLatticeAnnotation(k8sPolicy) if !ok { @@ -184,12 +212,13 @@ func (c *IAMAuthPolicyController) handleLatticeResourceChange(ctx context.Contex return nil } -func (c *IAMAuthPolicyController) updateLatticeAnnotaion(k8sPolicy *anv1alpha1.IAMAuthPolicy, resId, resType string) { +func (c *IAMAuthPolicyController) updateLatticeAnnotaion(k8sPolicy *anv1alpha1.IAMAuthPolicy, resId, resType string, resName string) { if k8sPolicy.Annotations == nil { k8sPolicy.Annotations = make(map[string]string) } k8sPolicy.Annotations[IAMAuthPolicyAnnotationResId] = resId k8sPolicy.Annotations[IAMAuthPolicyAnnotationType] = resType + k8sPolicy.Annotations[IAMAuthPolicyAnnotationResName] = resName } func (c *IAMAuthPolicyController) getLatticeAnnotation(k8sPolicy *anv1alpha1.IAMAuthPolicy) (model.IAMAuthPolicy, bool) { @@ -198,11 +227,45 @@ func (c *IAMAuthPolicyController) getLatticeAnnotation(k8sPolicy *anv1alpha1.IAM } resourceId := k8sPolicy.Annotations[IAMAuthPolicyAnnotationResId] resourceType := k8sPolicy.Annotations[IAMAuthPolicyAnnotationType] + resourceName := k8sPolicy.Annotations[IAMAuthPolicyAnnotationResName] if resourceId == "" || resourceType == "" { return model.IAMAuthPolicy{}, false } return model.IAMAuthPolicy{ Type: resourceType, ResourceId: resourceId, + Name: resourceName, }, true } + +func (c *IAMAuthPolicyController) targetRefToResourceName( + ctx context.Context, + k8sPolicy *anv1alpha1.IAMAuthPolicy, +) (string, error) { + targetRef := k8sPolicy.Spec.TargetRef + + if targetRef.Kind == "Gateway" { + return string(targetRef.Name), nil + } + + if targetRef.Kind == "HTTPRoute" || targetRef.Kind == "GRPCRoute" { + namespace := k8sPolicy.Namespace + if targetRef.Namespace != nil { + namespace = string(*targetRef.Namespace) + } + + targetRefObj, err := c.ph.GetTargetRefObj(ctx, k8sPolicy) + if err != nil { + return "", fmt.Errorf("failed to get target route: %w", err) + } + + serviceNameOverride, err := k8s.GetServiceNameOverrideWithValidation(targetRefObj) + if err != nil { + return "", fmt.Errorf("route '%s' has %w", targetRef.Name, err) + } + + return utils.LatticeServiceName(string(targetRef.Name), namespace, serviceNameOverride), nil + } + + return "", fmt.Errorf("unsupported targetRef kind: %s", targetRef.Kind) +} diff --git a/pkg/controllers/route_controller.go b/pkg/controllers/route_controller.go index 56b65d03..849bab93 100644 --- a/pkg/controllers/route_controller.go +++ b/pkg/controllers/route_controller.go @@ -192,7 +192,11 @@ func (r *routeReconciler) reconcileDelete(ctx context.Context, req ctrl.Request, k8s.RouteEventReasonReconcile, "Deleting Reconcile") if _, err := r.buildAndDeployModel(ctx, route); err != nil { - return fmt.Errorf("failed to cleanup route %s, %s: %w", route.Name(), route.Namespace(), err) + if k8s.IsInvalidServiceNameOverrideError(err) { + r.log.Infof(ctx, "Route %s deletion proceeding, no VPC Lattice resources created due to error: %v", route.Name(), err) + } else { + return fmt.Errorf("failed to cleanup route %s, %s: %w", route.Name(), route.Namespace(), err) + } } if err := updateRouteListenerStatus(ctx, r.client, route); err != nil { @@ -369,7 +373,12 @@ func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request, } func (r *routeReconciler) updateRouteStatusWithServiceInfo(ctx context.Context, route core.Route) error { - svcName := k8sutils.LatticeServiceName(route.Name(), route.Namespace()) + serviceNameOverride, err := k8s.GetServiceNameOverrideWithValidation(route.K8sObject()) + if err != nil { + return err + } + + svcName := k8sutils.LatticeServiceName(route.Name(), route.Namespace(), serviceNameOverride) svc, err := r.cloud.Lattice().FindService(ctx, svcName) if err != nil && !services.IsNotFoundError(err) { return err diff --git a/pkg/deploy/lattice/access_log_subscription_synthesizer_test.go b/pkg/deploy/lattice/access_log_subscription_synthesizer_test.go index 6ba4793b..feb69082 100644 --- a/pkg/deploy/lattice/access_log_subscription_synthesizer_test.go +++ b/pkg/deploy/lattice/access_log_subscription_synthesizer_test.go @@ -37,7 +37,7 @@ func TestSynthesizeAccessLogSubscription(t *testing.T) { }, } - stack, accessLogSubscription, _ := builder.Build(context.Background(), input) + stack, accessLogSubscription, _ := builder.Build(context.Background(), input, "TestName") mockManager.EXPECT().Create(ctx, accessLogSubscription).Return(&lattice.AccessLogSubscriptionStatus{}, nil).Times(1) @@ -57,7 +57,7 @@ func TestSynthesizeAccessLogSubscription(t *testing.T) { }, } - stack, accessLogSubscription, _ := builder.Build(context.Background(), input) + stack, accessLogSubscription, _ := builder.Build(context.Background(), input, "TestName") mockManager.EXPECT().Create(ctx, accessLogSubscription).Return(nil, errors.New("mock error")).Times(1) @@ -82,7 +82,7 @@ func TestSynthesizeAccessLogSubscription(t *testing.T) { }, } - stack, accessLogSubscription, _ := builder.Build(context.Background(), input) + stack, accessLogSubscription, _ := builder.Build(context.Background(), input, "TestName") k8sClient.EXPECT().List(context.Background(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() mockManager.EXPECT().Update(ctx, accessLogSubscription).Return(&lattice.AccessLogSubscriptionStatus{}, nil).AnyTimes() @@ -108,7 +108,7 @@ func TestSynthesizeAccessLogSubscription(t *testing.T) { }, } - stack, accessLogSubscription, _ := builder.Build(context.Background(), input) + stack, accessLogSubscription, _ := builder.Build(context.Background(), input, "TestName") k8sClient.EXPECT().List(context.Background(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() mockManager.EXPECT().Update(ctx, accessLogSubscription).Return(nil, errors.New("mock error")).AnyTimes() @@ -135,7 +135,7 @@ func TestSynthesizeAccessLogSubscription(t *testing.T) { }, } - stack, accessLogSubscription, _ := builder.Build(context.Background(), input) + stack, accessLogSubscription, _ := builder.Build(context.Background(), input, "TestName") mockManager.EXPECT().Delete(ctx, accessLogSubscription.Status.Arn).Return(nil).Times(1) @@ -159,7 +159,7 @@ func TestSynthesizeAccessLogSubscription(t *testing.T) { }, } - stack, _, _ := builder.Build(context.Background(), input) + stack, _, _ := builder.Build(context.Background(), input, "TestName") mockManager.EXPECT().Delete(gomock.Any(), gomock.Any()).Return(nil).Times(0) @@ -185,7 +185,7 @@ func TestSynthesizeAccessLogSubscription(t *testing.T) { }, } - stack, accessLogSubscription, _ := builder.Build(context.Background(), input) + stack, accessLogSubscription, _ := builder.Build(context.Background(), input, "TestName") mockManager.EXPECT().Delete(ctx, accessLogSubscription.Status.Arn).Return(errors.New("mock error")).Times(1) diff --git a/pkg/deploy/lattice/service_synthesizer.go b/pkg/deploy/lattice/service_synthesizer.go index 55944ed8..b792ff98 100644 --- a/pkg/deploy/lattice/service_synthesizer.go +++ b/pkg/deploy/lattice/service_synthesizer.go @@ -40,7 +40,7 @@ func (s *serviceSynthesizer) Synthesize(ctx context.Context) error { var svcErr error for _, resService := range resServices { - svcName := utils.LatticeServiceName(resService.Spec.RouteName, resService.Spec.RouteNamespace) + svcName := utils.LatticeServiceName(resService.Spec.RouteName, resService.Spec.RouteNamespace, resService.Spec.ServiceNameOverride) s.log.Debugf(ctx, "Synthesizing service: %s", svcName) if resService.IsDeleted { err := s.serviceManager.Delete(ctx, resService) diff --git a/pkg/gateway/model_build_access_log_subscription.go b/pkg/gateway/model_build_access_log_subscription.go index bdd2803a..8afd72ec 100644 --- a/pkg/gateway/model_build_access_log_subscription.go +++ b/pkg/gateway/model_build_access_log_subscription.go @@ -11,12 +11,11 @@ import ( "github.com/aws/aws-application-networking-k8s/pkg/k8s" "github.com/aws/aws-application-networking-k8s/pkg/model/core" model "github.com/aws/aws-application-networking-k8s/pkg/model/lattice" - "github.com/aws/aws-application-networking-k8s/pkg/utils" "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" ) type AccessLogSubscriptionModelBuilder interface { - Build(ctx context.Context, alp *anv1alpha1.AccessLogPolicy) (core.Stack, *model.AccessLogSubscription, error) + Build(ctx context.Context, alp *anv1alpha1.AccessLogPolicy, targetRefName string) (core.Stack, *model.AccessLogSubscription, error) } type accessLogSubscriptionModelBuilder struct { @@ -34,6 +33,7 @@ func NewAccessLogSubscriptionModelBuilder(log gwlog.Logger, client client.Client func (b *accessLogSubscriptionModelBuilder) Build( ctx context.Context, accessLogPolicy *anv1alpha1.AccessLogPolicy, + targetRefName string, ) (core.Stack, *model.AccessLogSubscription, error) { stack := core.NewDefaultStack(core.StackID(k8s.NamespacedName(accessLogPolicy))) @@ -41,6 +41,7 @@ func (b *accessLogSubscriptionModelBuilder) Build( log: b.log, stack: stack, accessLogPolicy: accessLogPolicy, + targetRefName: targetRefName, } if err := task.run(ctx); err != nil { @@ -55,6 +56,7 @@ type accessLogSubscriptionModelBuildTask struct { stack core.Stack accessLogPolicy *anv1alpha1.AccessLogPolicy accessLogSubscription *model.AccessLogSubscription + targetRefName string } func (t *accessLogSubscriptionModelBuildTask) run(ctx context.Context) error { @@ -70,10 +72,7 @@ func (t *accessLogSubscriptionModelBuildTask) run(ctx context.Context) error { sourceType = model.ServiceNetworkSourceType } - sourceName, err := utils.TargetRefToLatticeResourceName(t.accessLogPolicy.Spec.TargetRef, t.accessLogPolicy.Namespace) - if err != nil && eventType != core.DeleteEvent { - return err - } + sourceName := t.targetRefName destinationArn := t.accessLogPolicy.Spec.DestinationArn if destinationArn == nil { @@ -107,7 +106,7 @@ func (t *accessLogSubscriptionModelBuildTask) run(ctx context.Context) error { alsSpec.AdditionalTags = k8s.GetAdditionalTagsFromAnnotations(ctx, t.accessLogPolicy) t.accessLogSubscription = model.NewAccessLogSubscription(t.stack, alsSpec, status) - err = t.stack.AddResource(t.accessLogSubscription) + err := t.stack.AddResource(t.accessLogSubscription) if err != nil { return err } diff --git a/pkg/gateway/model_build_access_log_subscription_test.go b/pkg/gateway/model_build_access_log_subscription_test.go index c953a49f..4d79e527 100644 --- a/pkg/gateway/model_build_access_log_subscription_test.go +++ b/pkg/gateway/model_build_access_log_subscription_test.go @@ -46,12 +46,14 @@ func Test_BuildAccessLogSubscription(t *testing.T) { tests := []struct { description string input *anv1alpha1.AccessLogPolicy + targetRefName string expectedOutput *lattice.AccessLogSubscription onlyCompareSpecs bool expectedError error }{ { - description: "Policy on Gateway without namespace maps to ALS on Service Network with Gateway name", + description: "Policy on Gateway uses passed targetRefName as ServiceNetwork SourceName", + targetRefName: name, input: &anv1alpha1.AccessLogPolicy{ ObjectMeta: apimachineryv1.ObjectMeta{ Namespace: namespace, @@ -78,35 +80,8 @@ func Test_BuildAccessLogSubscription(t *testing.T) { expectedError: nil, }, { - description: "Policy on Gateway with namespace maps to ALS on Service Network with Gateway name", - input: &anv1alpha1.AccessLogPolicy{ - ObjectMeta: apimachineryv1.ObjectMeta{ - Namespace: namespace, - Name: name, - }, - Spec: anv1alpha1.AccessLogPolicySpec{ - DestinationArn: aws.String(s3DestinationArn), - TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ - Kind: gatewayKind, - Name: name, - Namespace: (*gwv1alpha2.Namespace)(aws.String(namespace)), - }, - }, - }, - expectedOutput: &lattice.AccessLogSubscription{ - Spec: lattice.AccessLogSubscriptionSpec{ - SourceType: lattice.ServiceNetworkSourceType, - SourceName: name, - DestinationArn: s3DestinationArn, - ALPNamespacedName: expectedNamespacedName, - EventType: core.CreateEvent, - }, - }, - onlyCompareSpecs: true, - expectedError: nil, - }, - { - description: "Policy on HTTPRoute without namespace maps to ALS on Service with HTTPRoute name + Policy's namespace", + description: "Policy on HTTPRoute uses passed targetRefName as Service SourceName", + targetRefName: name, input: &anv1alpha1.AccessLogPolicy{ ObjectMeta: apimachineryv1.ObjectMeta{ Namespace: namespace, @@ -123,35 +98,7 @@ func Test_BuildAccessLogSubscription(t *testing.T) { expectedOutput: &lattice.AccessLogSubscription{ Spec: lattice.AccessLogSubscriptionSpec{ SourceType: lattice.ServiceSourceType, - SourceName: fmt.Sprintf("%s-%s", name, namespace), - DestinationArn: s3DestinationArn, - ALPNamespacedName: expectedNamespacedName, - EventType: core.CreateEvent, - }, - }, - onlyCompareSpecs: true, - expectedError: nil, - }, - { - description: "Policy on HTTPRoute with namespace maps to ALS on Service Network with HTTPRoute name + namespace", - input: &anv1alpha1.AccessLogPolicy{ - ObjectMeta: apimachineryv1.ObjectMeta{ - Namespace: namespace, - Name: name, - }, - Spec: anv1alpha1.AccessLogPolicySpec{ - DestinationArn: aws.String(s3DestinationArn), - TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ - Kind: httpRouteKind, - Name: name, - Namespace: (*gwv1alpha2.Namespace)(aws.String(namespace)), - }, - }, - }, - expectedOutput: &lattice.AccessLogSubscription{ - Spec: lattice.AccessLogSubscriptionSpec{ - SourceType: lattice.ServiceSourceType, - SourceName: fmt.Sprintf("%s-%s", name, namespace), + SourceName: name, DestinationArn: s3DestinationArn, ALPNamespacedName: expectedNamespacedName, EventType: core.CreateEvent, @@ -161,7 +108,8 @@ func Test_BuildAccessLogSubscription(t *testing.T) { expectedError: nil, }, { - description: "Policy on GRPCRoute without namespace maps to ALS on Service with GRPCRoute name + Policy's namespace", + description: "Policy on GRPCRoute uses passed targetRefName as Service SourceName", + targetRefName: name, input: &anv1alpha1.AccessLogPolicy{ ObjectMeta: apimachineryv1.ObjectMeta{ Namespace: namespace, @@ -178,35 +126,7 @@ func Test_BuildAccessLogSubscription(t *testing.T) { expectedOutput: &lattice.AccessLogSubscription{ Spec: lattice.AccessLogSubscriptionSpec{ SourceType: lattice.ServiceSourceType, - SourceName: fmt.Sprintf("%s-%s", name, namespace), - DestinationArn: s3DestinationArn, - ALPNamespacedName: expectedNamespacedName, - EventType: core.CreateEvent, - }, - }, - onlyCompareSpecs: true, - expectedError: nil, - }, - { - description: "Policy on GRPCRoute with namespace maps to ALS on Service Network with GRPCRoute name + namespace", - input: &anv1alpha1.AccessLogPolicy{ - ObjectMeta: apimachineryv1.ObjectMeta{ - Namespace: namespace, - Name: name, - }, - Spec: anv1alpha1.AccessLogPolicySpec{ - DestinationArn: aws.String(s3DestinationArn), - TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ - Kind: grpcRouteKind, - Name: name, - Namespace: (*gwv1alpha2.Namespace)(aws.String(namespace)), - }, - }, - }, - expectedOutput: &lattice.AccessLogSubscription{ - Spec: lattice.AccessLogSubscriptionSpec{ - SourceType: lattice.ServiceSourceType, - SourceName: fmt.Sprintf("%s-%s", name, namespace), + SourceName: name, DestinationArn: s3DestinationArn, ALPNamespacedName: expectedNamespacedName, EventType: core.CreateEvent, @@ -216,7 +136,8 @@ func Test_BuildAccessLogSubscription(t *testing.T) { expectedError: nil, }, { - description: "Policy on Gateway with deletion timestamp is marked as deleted", + description: "Policy on Gateway with deletion timestamp is marked as deleted", + targetRefName: name, input: &anv1alpha1.AccessLogPolicy{ ObjectMeta: apimachineryv1.ObjectMeta{ Namespace: namespace, @@ -247,7 +168,8 @@ func Test_BuildAccessLogSubscription(t *testing.T) { expectedError: nil, }, { - description: "Policy on Gateway with Access Log Subscription annotation present is marked as updated", + description: "Policy on Gateway with Access Log Subscription annotation present is marked as updated", + targetRefName: name, input: &anv1alpha1.AccessLogPolicy{ ObjectMeta: apimachineryv1.ObjectMeta{ Namespace: namespace, @@ -277,38 +199,8 @@ func Test_BuildAccessLogSubscription(t *testing.T) { expectedError: nil, }, { - description: "Delete event skips targetRef validation", - input: &anv1alpha1.AccessLogPolicy{ - ObjectMeta: apimachineryv1.ObjectMeta{ - Namespace: namespace, - Name: name, - DeletionTimestamp: &apimachineryv1.Time{}, - Annotations: map[string]string{ - anv1alpha1.AccessLogSubscriptionAnnotationKey: "arn:aws:vpc-lattice:us-west-2:123456789012:accesslogsubscription/als-12345678901234567", - }, - }, - Spec: anv1alpha1.AccessLogPolicySpec{ - DestinationArn: aws.String(s3DestinationArn), - TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ - Kind: "Foo", - Name: name, - }, - }, - }, - expectedOutput: &lattice.AccessLogSubscription{ - Spec: lattice.AccessLogSubscriptionSpec{ - SourceType: lattice.ServiceSourceType, - SourceName: "", - DestinationArn: s3DestinationArn, - ALPNamespacedName: expectedNamespacedName, - EventType: core.DeleteEvent, - }, - }, - onlyCompareSpecs: true, - expectedError: nil, - }, - { - description: "Delete event skips destinationArn validation", + description: "Delete event skips destinationArn validation", + targetRefName: name, input: &anv1alpha1.AccessLogPolicy{ ObjectMeta: apimachineryv1.ObjectMeta{ Namespace: namespace, @@ -341,7 +233,7 @@ func Test_BuildAccessLogSubscription(t *testing.T) { for _, tt := range tests { fmt.Printf("Testing: %s\n", tt.description) - _, als, err := modelBuilder.Build(ctx, tt.input) + _, als, err := modelBuilder.Build(ctx, tt.input, tt.targetRefName) if tt.onlyCompareSpecs { assert.Equal(t, tt.expectedOutput.Spec, als.Spec, tt.description) } else { @@ -411,7 +303,7 @@ func Test_BuildAccessLogSubscription_WithAndWithoutAdditionalTagsAnnotation(t *t for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, als, err := modelBuilder.Build(ctx, tt.input) + _, als, err := modelBuilder.Build(ctx, tt.input, name) assert.NoError(t, err, tt.description) assert.Equal(t, tt.expectedAdditionalTags, als.Spec.AdditionalTags, tt.description) diff --git a/pkg/gateway/model_build_lattice_service.go b/pkg/gateway/model_build_lattice_service.go index ff471f6e..e2a84733 100644 --- a/pkg/gateway/model_build_lattice_service.go +++ b/pkg/gateway/model_build_lattice_service.go @@ -195,6 +195,12 @@ func (t *latticeServiceModelBuildTask) buildLatticeService(ctx context.Context) } spec.AllowTakeoverFrom = allowTakeoverFrom + serviceNameOverride, err := k8s.GetServiceNameOverrideWithValidation(t.route.K8sObject()) + if err != nil { + return nil, err + } + spec.ServiceNameOverride = serviceNameOverride + svc, err := model.NewLatticeService(t.stack, spec) if err != nil { return nil, err diff --git a/pkg/k8s/policyhelper/policy.go b/pkg/k8s/policyhelper/policy.go index 11ae781b..506754f7 100644 --- a/pkg/k8s/policyhelper/policy.go +++ b/pkg/k8s/policyhelper/policy.go @@ -430,3 +430,7 @@ func (h *PolicyHandler[P]) conflictResolutionSort(policies []P) { } }) } + +func (h *PolicyHandler[P]) GetTargetRefObj(ctx context.Context, policy P) (k8sclient.Object, error) { + return h.client.TargetRefObj(ctx, policy) +} diff --git a/pkg/k8s/utils.go b/pkg/k8s/utils.go index 991732be..7f550753 100644 --- a/pkg/k8s/utils.go +++ b/pkg/k8s/utils.go @@ -2,6 +2,7 @@ package k8s import ( "context" + "errors" "fmt" "regexp" "strings" @@ -10,6 +11,7 @@ import ( "github.com/aws/aws-application-networking-k8s/pkg/model/core" "github.com/aws/aws-sdk-go/aws" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/discovery" @@ -35,6 +37,9 @@ const ( // HttpRoute takeover annotation AllowTakeoverFromAnnotation = AnnotationPrefix + "allow-takeover-from" + // Override lattice service generated by controller + ServiceNameOverrideAnnotation = AnnotationPrefix + "service-name-override" + // AWS tag validation limits maxTagKeyLength = 128 maxTagValueLength = 256 @@ -426,3 +431,70 @@ func GetNonAWSManagedTags(tags Tags) Tags { } return nonAWSManagedTags } + +func GetServiceNameOverrideWithValidation(route metav1.Object) (string, error) { + annotations := route.GetAnnotations() + if annotations == nil { + return "", nil + } + + serviceNameOverride := annotations[ServiceNameOverrideAnnotation] + if serviceNameOverride == "" { + return "", nil + } + + if err := ValidateVPCLatticeServiceName(serviceNameOverride); err != nil { + return "", NewInvalidServiceNameOverrideError(serviceNameOverride, err.Error()) + } + + return serviceNameOverride, nil +} + +func ValidateVPCLatticeServiceName(name string) error { + if len(name) < 3 { + return fmt.Errorf("service name must be at least 3 characters long") + } + if len(name) > 40 { + return fmt.Errorf("service name cannot exceed 40 characters") + } + + if !regexp.MustCompile(`^[a-z0-9-]+$`).MatchString(name) { + return fmt.Errorf("service name must contain only lowercase letters, numbers, and hyphens") + } + + if strings.HasPrefix(name, "svc-") { + return fmt.Errorf("service name cannot start with 'svc-'") + } + + if strings.HasPrefix(name, "-") { + return fmt.Errorf("service name cannot start with a hyphen") + } + + if strings.HasSuffix(name, "-") { + return fmt.Errorf("service name cannot end with a hyphen") + } + + if strings.Contains(name, "--") { + return fmt.Errorf("service name cannot contain double hyphens") + } + + return nil +} + +type InvalidServiceNameOverride struct { + ServiceName string + Message string +} + +func (e *InvalidServiceNameOverride) Error() string { + return fmt.Sprintf("invalid service name override '%s': %s", e.ServiceName, e.Message) +} + +func NewInvalidServiceNameOverrideError(serviceName string, message string) error { + return &InvalidServiceNameOverride{serviceName, message} +} + +func IsInvalidServiceNameOverrideError(err error) bool { + invalidServiceNameErr := &InvalidServiceNameOverride{} + return errors.As(err, &invalidServiceNameErr) +} diff --git a/pkg/k8s/utils_test.go b/pkg/k8s/utils_test.go index e5d13497..29a01264 100644 --- a/pkg/k8s/utils_test.go +++ b/pkg/k8s/utils_test.go @@ -2,6 +2,8 @@ package k8s import ( "context" + "errors" + "fmt" "strings" "testing" @@ -1455,3 +1457,223 @@ func TestGetAdditionalTagsFromAnnotations(t *testing.T) { }) } } + +func TestIsInvalidServiceNameOverrideError(t *testing.T) { + tests := []struct { + name string + err error + expected bool + description string + }{ + { + name: "InvalidServiceNameOverrideError type", + err: NewInvalidServiceNameOverrideError("test-service", "invalid name"), + expected: true, + description: "should return true for InvalidServiceNameOverrideError", + }, + { + name: "wrapped InvalidServiceNameOverrideError", + err: fmt.Errorf("wrapper: %w", NewInvalidServiceNameOverrideError("test-service", "invalid name")), + expected: true, + description: "should return true for wrapped InvalidServiceNameOverrideError", + }, + { + name: "regular fmt.Errorf", + err: fmt.Errorf("regular error"), + expected: false, + description: "should return false for regular errors", + }, + { + name: "nil error", + err: nil, + expected: false, + description: "should return false for nil error", + }, + { + name: "different error type", + err: errors.New("different error"), + expected: false, + description: "should return false for different error types", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := IsInvalidServiceNameOverrideError(tt.err) + assert.Equal(t, tt.expected, result, tt.description) + }) + } +} + +func TestValidateVPCLatticeServiceName(t *testing.T) { + tests := []struct { + name string + serviceName string + expectError bool + description string + }{ + { + name: "valid service name", + serviceName: "my-service-123", + expectError: false, + description: "should accept valid service name", + }, + { + name: "too short", + serviceName: "ab", + expectError: true, + description: "should reject service name shorter than 3 characters", + }, + { + name: "too long", + serviceName: strings.Repeat("a", 41), + expectError: true, + description: "should reject service name longer than 40 characters", + }, + { + name: "contains uppercase", + serviceName: "My-Service", + expectError: true, + description: "should reject service name with uppercase letters", + }, + { + name: "starts with svc-", + serviceName: "svc-myservice", + expectError: true, + description: "should reject service name starting with 'svc-' prefix", + }, + { + name: "contains double hyphens", + serviceName: "my--service", + expectError: true, + description: "should reject service name with double hyphens", + }, + { + name: "starts with hyphen", + serviceName: "-myservice", + expectError: true, + description: "should reject service name starting with hyphen", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateVPCLatticeServiceName(tt.serviceName) + if tt.expectError { + assert.Error(t, err, tt.description) + } else { + assert.NoError(t, err, tt.description) + } + }) + } +} + +func TestGetServiceNameOverrideWithValidation(t *testing.T) { + tests := []struct { + name string + obj metav1.Object + expected string + expectError bool + description string + }{ + { + name: "no annotations", + obj: &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "default", + }, + }, + expected: "", + expectError: false, + description: "should return empty string when no annotations", + }, + { + name: "empty annotations map", + obj: &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "default", + Annotations: map[string]string{}, + }, + }, + expected: "", + expectError: false, + description: "should return empty string when annotations map is empty", + }, + { + name: "no service name override annotation", + obj: &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "default", + Annotations: map[string]string{ + "other-annotation": "value", + }, + }, + }, + expected: "", + expectError: false, + description: "should return empty string when service name override annotation not present", + }, + { + name: "empty service name override", + obj: &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "default", + Annotations: map[string]string{ + ServiceNameOverrideAnnotation: "", + }, + }, + }, + expected: "", + expectError: false, + description: "should return empty string when service name override is empty", + }, + { + name: "valid service name override", + obj: &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "default", + Annotations: map[string]string{ + ServiceNameOverrideAnnotation: "my-custom-service", + }, + }, + }, + expected: "my-custom-service", + expectError: false, + description: "should return valid service name override", + }, + { + name: "invalid service name override validation fails", + obj: &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-route", + Namespace: "default", + Annotations: map[string]string{ + ServiceNameOverrideAnnotation: "ab", + }, + }, + }, + expected: "", + expectError: true, + description: "should return error when ValidateVPCLatticeServiceName fails", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := GetServiceNameOverrideWithValidation(tt.obj) + + if tt.expectError { + assert.Error(t, err, tt.description) + assert.True(t, IsInvalidServiceNameOverrideError(err), "Should return InvalidServiceNameError for validation failures") + } else { + assert.NoError(t, err, tt.description) + } + assert.Equal(t, tt.expected, result, tt.description) + }) + } +} diff --git a/pkg/model/lattice/iamauthpolicy.go b/pkg/model/lattice/iamauthpolicy.go index 845e9e2a..782aae2f 100644 --- a/pkg/model/lattice/iamauthpolicy.go +++ b/pkg/model/lattice/iamauthpolicy.go @@ -4,7 +4,6 @@ import ( "fmt" anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1" - "github.com/aws/aws-application-networking-k8s/pkg/utils" ) type IAMAuthPolicy struct { @@ -18,20 +17,20 @@ type IAMAuthPolicyStatus struct { ResourceId string } -func NewIAMAuthPolicy(k8sPolicy *anv1alpha1.IAMAuthPolicy) IAMAuthPolicy { +func NewIAMAuthPolicy(k8sPolicy *anv1alpha1.IAMAuthPolicy, name string) IAMAuthPolicy { kind := k8sPolicy.Spec.TargetRef.Kind policy := k8sPolicy.Spec.Policy switch kind { case "Gateway": return IAMAuthPolicy{ Type: ServiceNetworkType, - Name: string(k8sPolicy.Spec.TargetRef.Name), + Name: name, Policy: policy, } case "HTTPRoute", "GRPCRoute": return IAMAuthPolicy{ Type: ServiceType, - Name: utils.LatticeServiceName(string(k8sPolicy.Spec.TargetRef.Name), k8sPolicy.Namespace), + Name: name, Policy: policy, } default: diff --git a/pkg/model/lattice/service.go b/pkg/model/lattice/service.go index 250bdd3c..0b2d5ad7 100644 --- a/pkg/model/lattice/service.go +++ b/pkg/model/lattice/service.go @@ -20,6 +20,7 @@ type ServiceSpec struct { CustomerCertARN string `json:"customercertarn"` AdditionalTags services.Tags `json:"additionaltags,omitempty"` AllowTakeoverFrom string `json:"allowtakeoverfrom,omitempty"` + ServiceNameOverride string `json:"servicenameoverride,omitempty"` } type ServiceStatus struct { @@ -73,5 +74,5 @@ func (s *Service) LatticeServiceName() string { } func (s *ServiceSpec) LatticeServiceName() string { - return utils.LatticeServiceName(s.RouteName, s.RouteNamespace) + return utils.LatticeServiceName(s.RouteName, s.RouteNamespace, s.ServiceNameOverride) } diff --git a/pkg/utils/common.go b/pkg/utils/common.go index c6f0582e..3bca5daa 100644 --- a/pkg/utils/common.go +++ b/pkg/utils/common.go @@ -6,7 +6,6 @@ import ( "strings" "golang.org/x/exp/constraints" - gwv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" ) type MapFunc[T any, U any] func(T) U @@ -60,30 +59,12 @@ func SliceFilter[T any](in []T, f FilterFunc[T]) []T { return out } -func LatticeServiceName(k8sSourceRouteName string, k8sSourceRouteNamespace string) string { - return fmt.Sprintf("%s-%s", Truncate(k8sSourceRouteName, 20), Truncate(k8sSourceRouteNamespace, 18)) -} - -func TargetRefToLatticeResourceName( - targetRef *gwv1alpha2.NamespacedPolicyTargetReference, - parentNamespace string, -) (string, error) { - // For Service Network, the name is just the Gateway's name. - if targetRef.Kind == "Gateway" { - return string(targetRef.Name), nil +func LatticeServiceName(k8sSourceRouteName string, k8sSourceRouteNamespace string, serviceNameOverride string) string { + if serviceNameOverride != "" { + return serviceNameOverride } - // For VPC Lattice Service, the name is Route's name, followed by hyphen (-), then the Route's namespace. - // If the Route's namespace is not provided, we assume it's the parent's namespace. - if targetRef.Kind == "HTTPRoute" || targetRef.Kind == "GRPCRoute" { - namespace := parentNamespace - if targetRef.Namespace != nil { - namespace = string(*targetRef.Namespace) - } - return LatticeServiceName(string(targetRef.Name), namespace), nil - } - - return "", fmt.Errorf("unsupported targetRef Kind: %s", targetRef.Kind) + return fmt.Sprintf("%s-%s", Truncate(k8sSourceRouteName, 20), Truncate(k8sSourceRouteNamespace, 18)) } func RandomAlphaString(length int) string { diff --git a/test/pkg/test/framework.go b/test/pkg/test/framework.go index a940e6d2..a10f9a8b 100644 --- a/test/pkg/test/framework.go +++ b/test/pkg/test/framework.go @@ -9,6 +9,7 @@ import ( "time" anaws "github.com/aws/aws-application-networking-k8s/pkg/aws" + "github.com/aws/aws-application-networking-k8s/pkg/k8s" "github.com/aws/aws-sdk-go/service/ec2" "github.com/onsi/gomega/format" @@ -320,7 +321,8 @@ func (env *Framework) GetServiceNetwork(ctx context.Context, gateway *gwv1.Gatew func (env *Framework) GetVpcLatticeService(ctx context.Context, route core.Route) *vpclattice.ServiceSummary { var found *vpclattice.ServiceSummary - latticeServiceName := utils.LatticeServiceName(route.Name(), route.Namespace()) + serviceNameOverride, _ := k8s.GetServiceNameOverrideWithValidation(route.K8sObject()) + latticeServiceName := utils.LatticeServiceName(route.Name(), route.Namespace(), serviceNameOverride) Eventually(func(g Gomega) { svc, err := env.LatticeClient.FindService(ctx, latticeServiceName) g.Expect(err).ToNot(HaveOccurred()) diff --git a/test/suites/integration/access_log_policy_test.go b/test/suites/integration/access_log_policy_test.go index 7f9da1be..d2489a8c 100644 --- a/test/suites/integration/access_log_policy_test.go +++ b/test/suites/integration/access_log_policy_test.go @@ -68,23 +68,29 @@ var _ = Describe("Access Log Policy", Ordered, func() { ) var ( - s3Client *s3.S3 - logsClient *cloudwatchlogs.CloudWatchLogs - firehoseClient *firehose.Firehose - iamClient *iam.IAM - httpDeployment *appsv1.Deployment - grpcDeployment *appsv1.Deployment - httpK8sService *corev1.Service - grpcK8sService *corev1.Service - httpRoute *gwv1.HTTPRoute - grpcRoute *gwv1.GRPCRoute - bucketArn string - logGroupArn string - logGroup2Arn string - deliveryStreamArn string - roleArn string - awsResourceName string - sess = session.Must(session.NewSession(&aws.Config{Region: aws.String(config.Region)})) + s3Client *s3.S3 + logsClient *cloudwatchlogs.CloudWatchLogs + firehoseClient *firehose.Firehose + iamClient *iam.IAM + httpDeployment *appsv1.Deployment + grpcDeployment *appsv1.Deployment + httpK8sService *corev1.Service + grpcK8sService *corev1.Service + httpRoute *gwv1.HTTPRoute + grpcRoute *gwv1.GRPCRoute + httpRouteWithInvalidServiceNameOverride *gwv1.HTTPRoute + httpDeploymentWithInvalidServiceNameOverride *appsv1.Deployment + httpSvcWithInvalidServiceNameOverride *corev1.Service + httpRouteWithValidServiceNameOverride *gwv1.HTTPRoute + httpDeploymentWithValidServiceNameOverride *appsv1.Deployment + httpSvcWithValidServiceNameOverride *corev1.Service + bucketArn string + logGroupArn string + logGroup2Arn string + deliveryStreamArn string + roleArn string + awsResourceName string + sess = session.Must(session.NewSession(&aws.Config{Region: aws.String(config.Region)})) ) BeforeAll(func() { @@ -215,6 +221,30 @@ var _ = Describe("Access Log Policy", Ordered, func() { } grpcRoute = testFramework.NewGRPCRoute(k8snamespace, testGateway, grpcRouteRules) testFramework.ExpectCreated(ctx, grpcRoute, grpcDeployment, grpcK8sService) + + httpDeploymentWithInvalidServiceNameOverride, httpSvcWithInvalidServiceNameOverride = testFramework.NewHttpApp(test.HTTPAppOptions{ + Name: "invalid-service-name-override-test", + Namespace: k8snamespace, + }) + + httpRouteWithInvalidServiceNameOverride = testFramework.NewHttpRoute(testGateway, httpSvcWithInvalidServiceNameOverride, "Service") + httpRouteWithInvalidServiceNameOverride.Annotations = map[string]string{ + "application-networking.k8s.aws/service-name-override": "svc-my-awesome-service", + } + + httpDeploymentWithValidServiceNameOverride, httpSvcWithValidServiceNameOverride = testFramework.NewHttpApp(test.HTTPAppOptions{ + Name: "valid-service-name-override-test", + Namespace: k8snamespace, + }) + + httpRouteWithValidServiceNameOverride = testFramework.NewHttpRoute(testGateway, httpSvcWithValidServiceNameOverride, "Service") + httpRouteWithValidServiceNameOverride.Annotations = map[string]string{ + "application-networking.k8s.aws/service-name-override": "my-awesome-service", + } + + testFramework.ExpectCreated(ctx, + httpDeploymentWithInvalidServiceNameOverride, httpSvcWithInvalidServiceNameOverride, httpRouteWithInvalidServiceNameOverride, + httpDeploymentWithValidServiceNameOverride, httpSvcWithValidServiceNameOverride, httpRouteWithValidServiceNameOverride) }) It("creation produces an Access Log Subscription for the corresponding Service Network when the targetRef's Kind is Gateway", func() { @@ -1361,6 +1391,114 @@ var _ = Describe("Access Log Policy", Ordered, func() { }).Should(Succeed()) }) + It("creates Access Log Subscription for HTTPRoute with service name override", func() { + accessLogPolicy := &anv1alpha1.AccessLogPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "access-log-policy-service-name-override", + Namespace: k8snamespace, + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + DestinationArn: aws.String(bucketArn), + TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ + Group: gwv1.GroupName, + Kind: "HTTPRoute", + Name: gwv1alpha2.ObjectName(httpRouteWithValidServiceNameOverride.Name), + Namespace: (*gwv1alpha2.Namespace)(aws.String(k8snamespace)), + }, + }, + } + testFramework.ExpectCreated(ctx, accessLogPolicy) + + Eventually(func(g Gomega) { + alpNamespacedName := types.NamespacedName{ + Name: accessLogPolicy.Name, + Namespace: accessLogPolicy.Namespace, + } + alp := &anv1alpha1.AccessLogPolicy{} + err := testFramework.Client.Get(ctx, alpNamespacedName, alp) + g.Expect(err).To(BeNil()) + g.Expect(len(alp.Status.Conditions)).To(BeEquivalentTo(1)) + g.Expect(alp.Status.Conditions[0].Type).To(BeEquivalentTo(string(gwv1alpha2.PolicyConditionAccepted))) + g.Expect(alp.Status.Conditions[0].Status).To(BeEquivalentTo(metav1.ConditionTrue)) + g.Expect(alp.Status.Conditions[0].Reason).To(BeEquivalentTo(string(gwv1alpha2.PolicyReasonAccepted))) + + customLatticeService := testFramework.GetVpcLatticeService(ctx, core.NewHTTPRoute(*httpRouteWithValidServiceNameOverride)) + g.Expect(*customLatticeService.Name).To(Equal("my-awesome-service")) + + listALSInput := &vpclattice.ListAccessLogSubscriptionsInput{ + ResourceIdentifier: customLatticeService.Arn, + } + listALSOutput, err := testFramework.LatticeClient.ListAccessLogSubscriptionsWithContext(ctx, listALSInput) + g.Expect(err).To(BeNil()) + g.Expect(len(listALSOutput.Items)).To(BeEquivalentTo(1)) + g.Expect(listALSOutput.Items[0].ResourceId).To(BeEquivalentTo(customLatticeService.Id)) + g.Expect(*listALSOutput.Items[0].DestinationArn).To(BeEquivalentTo(bucketArn)) + + g.Expect(alp.Annotations[anv1alpha1.AccessLogSubscriptionAnnotationKey]).To(BeEquivalentTo(*listALSOutput.Items[0].Arn)) + + g.Expect(alp.Annotations[anv1alpha1.AccessLogSubscriptionResourceNameAnnotationKey]).To(Equal("my-awesome-service")) + }).Should(Succeed()) + }) + + It("supports changing targetRef HTTPRoute with invalid service name override to valid service name override", func() { + accessLogPolicy := &anv1alpha1.AccessLogPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "access-log-policy-recovery-test", + Namespace: k8snamespace, + }, + Spec: anv1alpha1.AccessLogPolicySpec{ + DestinationArn: aws.String(bucketArn), + TargetRef: &gwv1alpha2.NamespacedPolicyTargetReference{ + Group: gwv1.GroupName, + Kind: "HTTPRoute", + Name: gwv1alpha2.ObjectName(httpRouteWithInvalidServiceNameOverride.Name), + Namespace: (*gwv1alpha2.Namespace)(aws.String(k8snamespace)), + }, + }, + } + testFramework.ExpectCreated(ctx, accessLogPolicy) + + Eventually(func(g Gomega) { + alpNamespacedName := types.NamespacedName{ + Name: accessLogPolicy.Name, + Namespace: accessLogPolicy.Namespace, + } + alp := &anv1alpha1.AccessLogPolicy{} + err := testFramework.Client.Get(ctx, alpNamespacedName, alp) + g.Expect(err).To(BeNil()) + + g.Expect(len(alp.Status.Conditions)).To(BeEquivalentTo(1)) + g.Expect(alp.Status.Conditions[0].Type).To(BeEquivalentTo(string(gwv1alpha2.PolicyConditionAccepted))) + g.Expect(alp.Status.Conditions[0].Status).To(BeEquivalentTo(metav1.ConditionFalse)) + g.Expect(alp.Status.Conditions[0].Reason).To(BeEquivalentTo(string(gwv1alpha2.PolicyReasonInvalid))) + }).Should(Succeed()) + + err := testFramework.Get(ctx, client.ObjectKeyFromObject(accessLogPolicy), accessLogPolicy) + Expect(err).To(BeNil()) + accessLogPolicy.Spec.TargetRef.Name = gwv1alpha2.ObjectName(httpRouteWithValidServiceNameOverride.Name) + testFramework.ExpectUpdated(ctx, accessLogPolicy) + + Eventually(func(g Gomega) { + alpNamespacedName := types.NamespacedName{ + Name: accessLogPolicy.Name, + Namespace: accessLogPolicy.Namespace, + } + alp := &anv1alpha1.AccessLogPolicy{} + err := testFramework.Client.Get(ctx, alpNamespacedName, alp) + g.Expect(err).To(BeNil()) + + g.Expect(len(alp.Status.Conditions)).To(BeEquivalentTo(1)) + g.Expect(alp.Status.Conditions[0].Type).To(BeEquivalentTo(string(gwv1alpha2.PolicyConditionAccepted))) + g.Expect(alp.Status.Conditions[0].Status).To(BeEquivalentTo(metav1.ConditionTrue)) + g.Expect(alp.Status.Conditions[0].Reason).To(BeEquivalentTo(string(gwv1alpha2.PolicyReasonAccepted))) + + validLatticeService := testFramework.GetVpcLatticeService(ctx, core.NewHTTPRoute(*httpRouteWithValidServiceNameOverride)) + g.Expect(*validLatticeService.Name).To(Equal("my-awesome-service")) + + g.Expect(alp.Annotations[anv1alpha1.AccessLogSubscriptionResourceNameAnnotationKey]).To(Equal("my-awesome-service")) + }).Should(Succeed()) + }) + AfterEach(func() { // Delete Access Log Policies in test namespace alps := &anv1alpha1.AccessLogPolicyList{} @@ -1376,10 +1514,16 @@ var _ = Describe("Access Log Policy", Ordered, func() { testFramework.ExpectDeletedThenNotFound(ctx, httpRoute, grpcRoute, + httpRouteWithInvalidServiceNameOverride, + httpRouteWithValidServiceNameOverride, httpK8sService, grpcK8sService, + httpSvcWithInvalidServiceNameOverride, + httpSvcWithValidServiceNameOverride, httpDeployment, grpcDeployment, + httpDeploymentWithInvalidServiceNameOverride, + httpDeploymentWithValidServiceNameOverride, ) // Find all AWS resources created in tests diff --git a/test/suites/integration/defined_target_ports_test.go b/test/suites/integration/defined_target_ports_test.go index d34477ad..08d081a1 100644 --- a/test/suites/integration/defined_target_ports_test.go +++ b/test/suites/integration/defined_target_ports_test.go @@ -63,7 +63,7 @@ var _ = Describe("Defined Target Ports", Ordered, func() { // Verify VPC Lattice Service exists route, _ := core.NewRoute(httpRoute) vpcLatticeService = testFramework.GetVpcLatticeService(ctx, route) - lsn := utils.LatticeServiceName(route.Name(), route.Namespace()) + lsn := utils.LatticeServiceName(route.Name(), route.Namespace(), "") Expect(*vpcLatticeService.DnsEntry).To(ContainSubstring(lsn)) performVerification(service, deployment, definedPorts) diff --git a/test/suites/integration/iamauthpolicy_test.go b/test/suites/integration/iamauthpolicy_test.go index 458e4ea4..9a80b3de 100644 --- a/test/suites/integration/iamauthpolicy_test.go +++ b/test/suites/integration/iamauthpolicy_test.go @@ -106,9 +106,15 @@ var _ = Describe("IAM Auth Policy", Ordered, func() { } var ( - httpDep *appsv1.Deployment - httpSvc *corev1.Service - httpRoute *gwv1.HTTPRoute + httpDep *appsv1.Deployment + httpSvc *corev1.Service + httpRoute *gwv1.HTTPRoute + httpDepWithServiceNameOverride *appsv1.Deployment + httpSvcWithServiceNameOverride *corev1.Service + httpRouteWithServiceNameOverride *gwv1.HTTPRoute + httpDepWithInvalidServiceNameOverride *appsv1.Deployment + httpSvcWithInvalidServiceNameOverride *corev1.Service + httpRouteWithInvalidServiceNameOverride *gwv1.HTTPRoute ) BeforeAll(func() { @@ -118,10 +124,33 @@ var _ = Describe("IAM Auth Policy", Ordered, func() { }) httpRoute = testFramework.NewHttpRoute(testGateway, httpSvc, "Service") testFramework.ExpectCreated(ctx, httpDep, httpSvc, httpRoute) + + httpDepWithServiceNameOverride, httpSvcWithServiceNameOverride = testFramework.NewHttpApp(test.HTTPAppOptions{ + Name: "iam-auth-override", + Namespace: k8snamespace, + }) + httpRouteWithServiceNameOverride = testFramework.NewHttpRoute(testGateway, httpSvcWithServiceNameOverride, "Service") + httpRouteWithServiceNameOverride.Annotations = map[string]string{ + "application-networking.k8s.aws/service-name-override": "my-awesome-service", + } + testFramework.ExpectCreated(ctx, httpDepWithServiceNameOverride, httpSvcWithServiceNameOverride, httpRouteWithServiceNameOverride) + + httpDepWithInvalidServiceNameOverride, httpSvcWithInvalidServiceNameOverride = testFramework.NewHttpApp(test.HTTPAppOptions{ + Name: "iam-auth-invalid", + Namespace: k8snamespace, + }) + httpRouteWithInvalidServiceNameOverride = testFramework.NewHttpRoute(testGateway, httpSvcWithInvalidServiceNameOverride, "Service") + httpRouteWithInvalidServiceNameOverride.Annotations = map[string]string{ + "application-networking.k8s.aws/service-name-override": "svc-my-awesome-service", + } + testFramework.ExpectCreated(ctx, httpDepWithInvalidServiceNameOverride, httpSvcWithInvalidServiceNameOverride, httpRouteWithInvalidServiceNameOverride) }) AfterAll(func() { - testFramework.ExpectDeletedThenNotFound(ctx, httpDep, httpSvc, httpRoute) + testFramework.ExpectDeletedThenNotFound(ctx, + httpDep, httpSvc, httpRoute, + httpDepWithServiceNameOverride, httpSvcWithServiceNameOverride, httpRouteWithServiceNameOverride, + httpDepWithInvalidServiceNameOverride, httpSvcWithInvalidServiceNameOverride, httpRouteWithInvalidServiceNameOverride) testFramework.ExpectDeleteAllToSucceed(ctx, &anv1alpha1.IAMAuthPolicy{}, k8snamespace) }) @@ -198,6 +227,120 @@ var _ = Describe("IAM Auth Policy", Ordered, func() { testLatticeSvcPolicy(svcId, vpclattice.AuthTypeNone, NoPolicy) log.Infof(ctx, "policy removed from Svc=%s", svcId) }) + + It("removes IAM AuthPolicy from old HTTPRoute when targetRef changes to new HTTPRoute", func() { + // Create second HTTPRoute for target change test + secondHttpDep, secondHttpSvc := testFramework.NewHttpApp(test.HTTPAppOptions{ + Name: SvcName + "-2", + Namespace: k8snamespace, + }) + secondHttpRoute := testFramework.NewHttpRoute(testGateway, secondHttpSvc, "Service") + secondHttpRoute.Name = SvcName + "-2" + testFramework.ExpectCreated(ctx, secondHttpSvc, secondHttpRoute) + + // Get VPC Lattice service IDs for both routes + firstSvc := testFramework.GetVpcLatticeService(ctx, core.NewHTTPRoute(gwv1.HTTPRoute(*httpRoute))) + secondSvc := testFramework.GetVpcLatticeService(ctx, core.NewHTTPRoute(gwv1.HTTPRoute(*secondHttpRoute))) + firstSvcId := *firstSvc.Id + secondSvcId := *secondSvc.Id + + // Create IAMAuthPolicy targeting first route + policy := newPolicy("target-change", "HTTPRoute", SvcName) + + // Verify policy applied to first service + wantResults := K8sResults{ + statusReason: gwv1alpha2.PolicyReasonAccepted, + annotationResType: model.ServiceType, + annotationResId: firstSvcId, + } + testK8sPolicy(policy, wantResults) + testLatticeSvcPolicy(firstSvcId, vpclattice.AuthTypeAwsIam, policy.Spec.Policy) + log.Infof(ctx, "policy initially applied for first Svc=%s", firstSvcId) + + // Change targetRef to second route + err := testFramework.Client.Get(ctx, client.ObjectKeyFromObject(policy), policy) + Expect(err).ToNot(HaveOccurred()) + policy.Spec.TargetRef.Name = gwv1.ObjectName(SvcName + "-2") + testFramework.Update(ctx, policy) + + wantResults = K8sResults{ + statusReason: gwv1alpha2.PolicyReasonAccepted, + annotationResType: model.ServiceType, + annotationResId: secondSvcId, + } + testK8sPolicy(policy, wantResults) + + // Verify new service has auth policy + testLatticeSvcPolicy(secondSvcId, vpclattice.AuthTypeAwsIam, policy.Spec.Policy) + log.Infof(ctx, "policy moved to second Svc=%s", secondSvcId) + + // Verify auth policy removed from old service + testLatticeSvcPolicy(firstSvcId, vpclattice.AuthTypeNone, NoPolicy) + log.Infof(ctx, "old policy cleaned up from first Svc=%s", firstSvcId) + + testFramework.ExpectDeletedThenNotFound(ctx, policy) + testLatticeSvcPolicy(secondSvcId, vpclattice.AuthTypeNone, NoPolicy) + testFramework.ExpectDeletedThenNotFound(ctx, secondHttpDep, secondHttpSvc, secondHttpRoute) + }) + + It("accepted, applied, and removed from HTTPRoute with service name override", func() { + policy := newPolicy("http-override", "HTTPRoute", httpRouteWithServiceNameOverride.Name) + svc := testFramework.GetVpcLatticeService(ctx, core.NewHTTPRoute(*httpRouteWithServiceNameOverride)) + svcId := *svc.Id + + Expect(*svc.Name).To(Equal("my-awesome-service")) + + wantResults := K8sResults{ + statusReason: gwv1alpha2.PolicyReasonAccepted, + annotationResType: model.ServiceType, + annotationResId: svcId, + } + testK8sPolicy(policy, wantResults) + + testLatticeSvcPolicy(svcId, vpclattice.AuthTypeAwsIam, policy.Spec.Policy) + + testFramework.ExpectDeletedThenNotFound(ctx, policy) + testLatticeSvcPolicy(svcId, vpclattice.AuthTypeNone, NoPolicy) + }) + + It("supports targetRef HTTPRoute change from invalid to valid service name override", func() { + policy := newPolicy("recovery-test", "HTTPRoute", httpRouteWithInvalidServiceNameOverride.Name) + + testK8sPolicy(policy, K8sResults{statusReason: gwv1alpha2.PolicyReasonInvalid}) + + err := testFramework.Client.Get(ctx, client.ObjectKeyFromObject(policy), policy) + Expect(err).ToNot(HaveOccurred()) + policy.Spec.TargetRef.Name = gwv1.ObjectName(httpRouteWithServiceNameOverride.Name) + testFramework.Update(ctx, policy) + + svc := testFramework.GetVpcLatticeService(ctx, core.NewHTTPRoute(*httpRouteWithServiceNameOverride)) + svcId := *svc.Id + Expect(*svc.Name).To(Equal("my-awesome-service")) + + wantResults := K8sResults{ + statusReason: gwv1alpha2.PolicyReasonAccepted, + annotationResType: model.ServiceType, + annotationResId: svcId, + } + + Eventually(func(g Gomega) (K8sResults, error) { + p := &anv1alpha1.IAMAuthPolicy{} + err := testFramework.Client.Get(ctx, client.ObjectKeyFromObject(policy), p) + if err != nil { + return K8sResults{}, err + } + return K8sResults{ + statusReason: GetPolicyStatusReason(p), + annotationResType: p.Annotations[controllers.IAMAuthPolicyAnnotationType], + annotationResId: p.Annotations[controllers.IAMAuthPolicyAnnotationResId], + }, nil + }).WithTimeout(120 * time.Second).WithPolling(time.Second). + Should(Equal(wantResults)) + testLatticeSvcPolicy(svcId, vpclattice.AuthTypeAwsIam, policy.Spec.Policy) + + testFramework.ExpectDeletedThenNotFound(ctx, policy) + testLatticeSvcPolicy(svcId, vpclattice.AuthTypeNone, NoPolicy) + }) }) type StatusConditionsReader interface { diff --git a/test/suites/integration/service_name_override_test.go b/test/suites/integration/service_name_override_test.go new file mode 100644 index 00000000..e786b68e --- /dev/null +++ b/test/suites/integration/service_name_override_test.go @@ -0,0 +1,197 @@ +package integration + +import ( + "strings" + "time" + + "github.com/aws/aws-sdk-go/service/vpclattice" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/aws/aws-application-networking-k8s/pkg/aws/services" + "github.com/aws/aws-application-networking-k8s/pkg/model/core" + "github.com/aws/aws-application-networking-k8s/test/pkg/test" + gwv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +var _ = Describe("Service Name Override", Ordered, func() { + It("HttpRoute creates VPC Lattice service with custom name when override specified", func() { + httpDeployment, httpSvc := testFramework.NewHttpApp(test.HTTPAppOptions{ + Name: "service-name-override-test", + Namespace: k8snamespace, + }) + + httpRoute := &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service-name-override-route", + Namespace: k8snamespace, + Annotations: map[string]string{ + "application-networking.k8s.aws/service-name-override": "custom-service-name", + }, + }, + Spec: gwv1.HTTPRouteSpec{ + CommonRouteSpec: gwv1.CommonRouteSpec{ + ParentRefs: []gwv1.ParentReference{ + { + Name: gwv1.ObjectName(testGateway.Name), + }, + }, + }, + Rules: []gwv1.HTTPRouteRule{ + { + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: gwv1.BackendRef{ + BackendObjectReference: gwv1.BackendObjectReference{ + Name: gwv1.ObjectName(httpSvc.Name), + Port: (*gwv1.PortNumber)(&httpSvc.Spec.Ports[0].Port), + }, + }, + }, + }, + }, + }, + }, + } + + testFramework.ExpectCreated(ctx, httpDeployment, httpSvc, httpRoute) + Eventually(func(g Gomega) { + route := core.NewHTTPRoute(*httpRoute) + vpcLatticeService := testFramework.GetVpcLatticeService(ctx, route) + + g.Expect(*vpcLatticeService.Name).To(Equal("custom-service-name")) + g.Expect(*vpcLatticeService.Status).To(Equal(vpclattice.ServiceStatusActive)) + g.Expect(*vpcLatticeService.DnsEntry.DomainName).To(ContainSubstring("custom-service-name")) + + targetGroup := testFramework.GetTargetGroup(ctx, httpSvc) + g.Expect(*targetGroup.Protocol).To(Equal("HTTP")) + + listeners, err := testFramework.LatticeClient.ListListeners(&vpclattice.ListListenersInput{ + ServiceIdentifier: vpcLatticeService.Id, + }) + g.Expect(err).To(BeNil()) + g.Expect(len(listeners.Items)).To(BeNumerically(">", 0)) + + for _, listener := range listeners.Items { + rules, err := testFramework.LatticeClient.ListRules(&vpclattice.ListRulesInput{ + ServiceIdentifier: vpcLatticeService.Id, + ListenerIdentifier: listener.Id, + }) + g.Expect(err).To(BeNil()) + g.Expect(len(rules.Items)).To(BeNumerically(">", 0)) + + foundForwardRule := false + for _, rule := range rules.Items { + if rule.IsDefault != nil && *rule.IsDefault { + continue + } + + ruleDetail, err := testFramework.LatticeClient.GetRule(&vpclattice.GetRuleInput{ + ServiceIdentifier: vpcLatticeService.Id, + ListenerIdentifier: listener.Id, + RuleIdentifier: rule.Id, + }) + g.Expect(err).To(BeNil()) + + if ruleDetail.Action != nil && ruleDetail.Action.Forward != nil { + for _, targetGroupWeight := range ruleDetail.Action.Forward.TargetGroups { + if *targetGroupWeight.TargetGroupIdentifier == *targetGroup.Id { + foundForwardRule = true + break + } + } + } + } + g.Expect(foundForwardRule).To(BeTrue(), "Expected to find a rule that forwards to target group %s", *targetGroup.Id) + } + + associations, err := testFramework.LatticeClient.ListServiceNetworkServiceAssociations(&vpclattice.ListServiceNetworkServiceAssociationsInput{ + ServiceIdentifier: vpcLatticeService.Id, + }) + g.Expect(err).To(BeNil()) + g.Expect(len(associations.Items)).To(BeNumerically(">", 0)) + + for _, association := range associations.Items { + g.Expect(*association.Status).To(Equal(vpclattice.ServiceNetworkServiceAssociationStatusActive)) + } + + targets := testFramework.GetTargets(ctx, targetGroup, httpDeployment) + g.Expect(targets).ToNot(BeEmpty()) + }).Should(Succeed()) + + testFramework.ExpectDeletedThenNotFound(ctx, httpRoute, httpDeployment, httpSvc) + }) + + It("rejects HTTPRoute with invalid service name override and creates no VPC Lattice resources", func() { + invalidDep, invalidSvc := testFramework.NewHttpApp(test.HTTPAppOptions{ + Name: "invalid-test", + Namespace: k8snamespace, + }) + + invalidRoute := &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "invalid-route", + Namespace: k8snamespace, + Annotations: map[string]string{ + "application-networking.k8s.aws/service-name-override": "INVALID-UPPERCASE", + }, + }, + Spec: gwv1.HTTPRouteSpec{ + CommonRouteSpec: gwv1.CommonRouteSpec{ + ParentRefs: []gwv1.ParentReference{ + { + Name: gwv1.ObjectName(testGateway.Name), + }, + }, + }, + Rules: []gwv1.HTTPRouteRule{ + { + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: gwv1.BackendRef{ + BackendObjectReference: gwv1.BackendObjectReference{ + Name: gwv1.ObjectName(invalidSvc.Name), + Port: (*gwv1.PortNumber)(&invalidSvc.Spec.Ports[0].Port), + }, + }, + }, + }, + }, + }, + }, + } + + testFramework.ExpectCreated(ctx, invalidDep, invalidSvc, invalidRoute) + + Eventually(func(g Gomega) { + _, err := testFramework.LatticeClient.FindService(ctx, "INVALID-UPPERCASE") + g.Expect(services.IsNotFoundError(err)).To(BeTrue()) + + targetGroups, err := testFramework.LatticeClient.ListTargetGroupsAsList(ctx, &vpclattice.ListTargetGroupsInput{}) + g.Expect(err).To(BeNil()) + for _, tg := range targetGroups { + g.Expect(*tg.Name).ToNot(ContainSubstring("invalid-test")) + } + + events := &v1.EventList{} + err = testFramework.List(ctx, events, client.InNamespace(k8snamespace)) + g.Expect(err).ToNot(HaveOccurred()) + + foundValidationError := false + for _, event := range events.Items { + if event.InvolvedObject.Name == invalidRoute.Name && + event.Reason == "FailedBuildModel" && + strings.Contains(event.Message, "invalid service name override") { + foundValidationError = true + break + } + } + g.Expect(foundValidationError).To(BeTrue(), "Expected FailedBuildModel event with validation error") + }).WithTimeout(30 * time.Second).WithPolling(2 * time.Second).Should(Succeed()) + + testFramework.ExpectDeletedThenNotFound(ctx, invalidRoute, invalidDep, invalidSvc) + }) +})