From 9a4e9e715dd59774500e53b271c8764163ceba83 Mon Sep 17 00:00:00 2001 From: Shawn Kaplan Date: Mon, 2 Oct 2023 17:53:12 -0700 Subject: [PATCH 1/6] Change unwanted error logs to debug, remove unwanted info logs, add route reconciler info logs, remove use of struct embedding in logs --- controllers/route_controller.go | 15 +- controllers/serviceexport_controller.go | 9 +- controllers/serviceimport_controller.go | 8 +- pkg/deploy/lattice/listener_manager.go | 2 - pkg/deploy/lattice/listener_synthesizer.go | 2 +- pkg/deploy/lattice/rule_synthesizer.go | 4 +- .../lattice/target_group_synthesizer.go | 131 ++++++++---------- pkg/gateway/model_build_lattice_service.go | 2 +- pkg/gateway/model_build_listener.go | 2 +- pkg/gateway/model_build_rule.go | 4 +- pkg/gateway/model_build_targetgroup.go | 3 +- 11 files changed, 81 insertions(+), 101 deletions(-) diff --git a/controllers/route_controller.go b/controllers/route_controller.go index a325c05d..0cec919f 100644 --- a/controllers/route_controller.go +++ b/controllers/route_controller.go @@ -176,6 +176,8 @@ func (r *RouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl } func (r *RouteReconciler) reconcile(ctx context.Context, req ctrl.Request) error { + r.log.Infow("reconcile", "name", req.Name) + route, err := r.getRoute(ctx, req) if err != nil { return client.IgnoreNotFound(err) @@ -194,7 +196,7 @@ func (r *RouteReconciler) reconcile(ctx context.Context, req ctrl.Request) error r.eventRecorder.Event(route.K8sObject(), corev1.EventTypeNormal, k8s.RouteEventReasonReconcile, "Deleting Reconcile") if err := r.cleanupRouteResources(ctx, route); err != nil { - return fmt.Errorf("failed to cleanup GRPCRoute %v, %v: %w", route.Name(), route.Namespace(), err) + return fmt.Errorf("failed to cleanup GRPCRoute %s, %s: %w", route.Name(), route.Namespace(), err) } err = updateRouteListenerStatus(ctx, r.client, route) if err != nil { @@ -206,6 +208,7 @@ func (r *RouteReconciler) reconcile(ctx context.Context, req ctrl.Request) error } // TODO delete metrics + r.log.Infow("reconciled", "name", req.Name) return nil } else { r.log.Infow("reconcile, adding or updating", "name", req.Name) @@ -255,7 +258,7 @@ func (r *RouteReconciler) cleanupRouteResources(ctx context.Context, route core. func (r *RouteReconciler) isRouteRelevant(ctx context.Context, route core.Route) bool { if len(route.Spec().ParentRefs()) == 0 { - r.log.Infof("Ignore Route which has no ParentRefs gateway %v ", route.Name()) + r.log.Infof("Ignore Route which has no ParentRefs gateway %s ", route.Name()) return false } @@ -284,16 +287,16 @@ func (r *RouteReconciler) isRouteRelevant(ctx context.Context, route core.Route) } if err := r.client.Get(ctx, gwClassName, gwClass); err != nil { - r.log.Infof("Ignore Route not controlled by any GatewayClass %v, %v", route.Name(), route.Namespace()) + r.log.Infof("Ignore Route not controlled by any GatewayClass %s, %s", route.Name(), route.Namespace()) return false } if gwClass.Spec.ControllerName == config.LatticeGatewayControllerName { - r.log.Infof("Found aws-vpc-lattice for Route for %v, %v", route.Name(), route.Namespace()) + r.log.Infof("Found aws-vpc-lattice for Route for %s, %s", route.Name(), route.Namespace()) return true } - r.log.Infof("Ignore non aws-vpc-lattice Route %v, %v", route.Name(), route.Namespace()) + r.log.Infof("Ignore non aws-vpc-lattice Route %s, %s", route.Name(), route.Namespace()) return false } @@ -334,7 +337,7 @@ func (r *RouteReconciler) buildAndDeployModel( func (r *RouteReconciler) reconcileRouteResource(ctx context.Context, route core.Route) error { if err := r.finalizerManager.AddFinalizers(ctx, route.K8sObject(), routeTypeToFinalizer[r.routeType]); err != nil { - r.eventRecorder.Event(route.K8sObject(), corev1.EventTypeWarning, k8s.RouteEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %v", err)) + r.eventRecorder.Event(route.K8sObject(), corev1.EventTypeWarning, k8s.RouteEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %s", err)) } backendRefIPFamiliesErr := r.validateBackendRefsIpFamilies(ctx, route) diff --git a/controllers/serviceexport_controller.go b/controllers/serviceexport_controller.go index d32b9ce5..1b17b3a5 100644 --- a/controllers/serviceexport_controller.go +++ b/controllers/serviceexport_controller.go @@ -157,7 +157,7 @@ func (r *ServiceExportReconciler) cleanupServiceExportResources(ctx context.Cont func (r *ServiceExportReconciler) reconcileServiceExportResources(ctx context.Context, srvExport *mcs_api.ServiceExport) error { if err := r.finalizerManager.AddFinalizers(ctx, srvExport, serviceExportFinalizer); err != nil { - r.eventRecorder.Event(srvExport, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %v", err)) + r.eventRecorder.Event(srvExport, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %s", err)) return errors.New("TODO") } @@ -172,11 +172,12 @@ func (r *ServiceExportReconciler) buildAndDeployModel( stack, targetGroup, err := r.modelBuilder.Build(ctx, srvExport) if err != nil { - r.log.Infof("Failed to buildAndDeployModel for service export %v\n", srvExport) + r.log.Debugf("Failed to buildAndDeployModel for service export %s-%s due to %s", + srvExport.Name, srvExport.Namespace, err) r.eventRecorder.Event(srvExport, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedBuildModel, - fmt.Sprintf("Failed BuildModel due to %v", err)) + fmt.Sprintf("Failed BuildModel due to %s", err)) // Build failed means the K8S serviceexport, service are NOT ready to be deployed to lattice // return nil to complete controller loop for current change. @@ -191,7 +192,7 @@ func (r *ServiceExportReconciler) buildAndDeployModel( if err := r.stackDeployer.Deploy(ctx, stack); err != nil { r.eventRecorder.Event(srvExport, corev1.EventTypeWarning, - k8s.ServiceExportEventReasonFailedDeployModel, fmt.Sprintf("Failed deploy model due to %v", err)) + k8s.ServiceExportEventReasonFailedDeployModel, fmt.Sprintf("Failed deploy model due to %s", err)) return nil, nil, err } diff --git a/controllers/serviceimport_controller.go b/controllers/serviceimport_controller.go index a06a678c..7903f94b 100644 --- a/controllers/serviceimport_controller.go +++ b/controllers/serviceimport_controller.go @@ -19,10 +19,9 @@ package controllers import ( "context" "fmt" + "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" - "github.com/aws/aws-application-networking-k8s/pkg/k8s" - "github.com/aws/aws-application-networking-k8s/pkg/latticestore" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" @@ -30,6 +29,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" mcs_api "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1" + + "github.com/aws/aws-application-networking-k8s/pkg/k8s" + "github.com/aws/aws-application-networking-k8s/pkg/latticestore" ) // ServiceImportReconciler reconciles a ServiceImport object @@ -104,7 +106,7 @@ func (r *ServiceImportReconciler) Reconcile(ctx context.Context, req ctrl.Reques // Handle add if err := r.finalizerManager.AddFinalizers(ctx, serviceImport, serviceImportFinalizer); err != nil { - r.eventRecorder.Event(serviceImport, corev1.EventTypeWarning, k8s.ServiceImportEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %v", err)) + r.eventRecorder.Event(serviceImport, corev1.EventTypeWarning, k8s.ServiceImportEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %s", err)) return ctrl.Result{}, nil } diff --git a/pkg/deploy/lattice/listener_manager.go b/pkg/deploy/lattice/listener_manager.go index afa19b47..c58541bb 100644 --- a/pkg/deploy/lattice/listener_manager.go +++ b/pkg/deploy/lattice/listener_manager.go @@ -145,8 +145,6 @@ func (d *defaultListenerManager) List(ctx context.Context, serviceID string) ([] return sdkListeners, err } - d.log.Debugf("Found listeners for service %v", resp.Items) - for _, r := range resp.Items { listener := vpclattice.ListenerSummary{ Arn: r.Arn, diff --git a/pkg/deploy/lattice/listener_synthesizer.go b/pkg/deploy/lattice/listener_synthesizer.go index 3792438d..87e6bf03 100644 --- a/pkg/deploy/lattice/listener_synthesizer.go +++ b/pkg/deploy/lattice/listener_synthesizer.go @@ -61,7 +61,7 @@ func (l *listenerSynthesizer) Synthesize(ctx context.Context) error { // handle delete sdkListeners, err := l.getSDKListeners(ctx) if err != nil { - l.log.Errorf("Failed to get SDK Listeners during Listener synthesis due to err %s", err) + l.log.Debugf("Failed to get SDK Listeners during Listener synthesis due to err %s", err) } for _, sdkListener := range sdkListeners { diff --git a/pkg/deploy/lattice/rule_synthesizer.go b/pkg/deploy/lattice/rule_synthesizer.go index e6864920..2e96c225 100644 --- a/pkg/deploy/lattice/rule_synthesizer.go +++ b/pkg/deploy/lattice/rule_synthesizer.go @@ -39,7 +39,7 @@ func (r *ruleSynthesizer) Synthesize(ctx context.Context) error { err := r.stack.ListResources(&resRule) if err != nil { - r.log.Debugf("Error while synthesizing rules %v, %s", resRule, err) + r.log.Debugf("Error while listing rules %s", err) } updatePriority := false @@ -61,7 +61,7 @@ func (r *ruleSynthesizer) Synthesize(ctx context.Context) error { // handle delete sdkRules, err := r.getSDKRules(ctx) if err != nil { - r.log.Debugf("Error while getting rules %v, %s", sdkRules, err) + r.log.Debugf("Error while getting rules due to %s", err) } for _, sdkRule := range sdkRules { diff --git a/pkg/deploy/lattice/target_group_synthesizer.go b/pkg/deploy/lattice/target_group_synthesizer.go index 5d3eb967..d0986daf 100644 --- a/pkg/deploy/lattice/target_group_synthesizer.go +++ b/pkg/deploy/lattice/target_group_synthesizer.go @@ -3,10 +3,12 @@ package lattice import ( "context" "errors" - "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" - "github.com/aws/aws-sdk-go/service/vpclattice" "strings" + "github.com/aws/aws-sdk-go/service/vpclattice" + + "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" mcs_api "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1" @@ -48,8 +50,6 @@ type TargetGroupSynthesizer struct { func (t *TargetGroupSynthesizer) Synthesize(ctx context.Context) error { var ret = "" - t.log.Infof("Started synthesizing TargetGroups") - if err := t.SynthesizeTriggeredTargetGroup(ctx); err != nil { ret = LATTICE_RETRY } @@ -80,8 +80,6 @@ func (t *TargetGroupSynthesizer) SynthesizeTriggeredTargetGroup(ctx context.Cont t.stack.ListResources(&resTargetGroups) - t.log.Infof("Synthesize TargetGroups ==[%v]", resTargetGroups) - for _, resTargetGroup := range resTargetGroups { // find out VPC for service import @@ -114,10 +112,10 @@ func (t *TargetGroupSynthesizer) SynthesizeTriggeredTargetGroup(ctx context.Cont //Ingnore TG delete since this is an import from elsewhere continue } - tgStatus, err := t.targetGroupManager.Get(ctx, resTargetGroup) + tgStatus, err := t.targetGroupManager.Get(ctx, resTargetGroup) if err != nil { - t.log.Infof("Error on t.targetGroupManager.Get for %v err %v", resTargetGroup, err) + t.log.Debugf("Error getting target group: %s", err) returnErr = true continue } @@ -128,27 +126,23 @@ func (t *TargetGroupSynthesizer) SynthesizeTriggeredTargetGroup(ctx context.Cont resTargetGroup.Spec.Config.VpcID, tgStatus.TargetGroupARN, tgStatus.TargetGroupID, resTargetGroup.Spec.Config.IsServiceImport, "") - t.log.Infof("targetGroup Synthesized successfully for %s: %v", resTargetGroup.Spec.Name, tgStatus) - + t.log.Infof("Successfully synthesized target group %s with status %s", resTargetGroup.Spec.Name, tgStatus) } else { if resTargetGroup.Spec.IsDeleted { err := t.targetGroupManager.Delete(ctx, resTargetGroup) - if err != nil { returnErr = true continue } else { - t.log.Infof("Synthesizing Target Group: successfully deleted target group %v", resTargetGroup) + t.log.Debugf("Successfully deleted target group %s", resTargetGroup.Spec.Name) t.latticeDataStore.DelTargetGroup(resTargetGroup.Spec.Name, resTargetGroup.Spec.Config.K8SHTTPRouteName, false) } - } else { resTargetGroup.Spec.Config.VpcID = config.VpcID tgStatus, err := t.targetGroupManager.Create(ctx, resTargetGroup) - if err != nil { - t.log.Infof("Error on t.targetGroupManager.Create for %v err %v", resTargetGroup, err) + t.log.Debugf("Error creating target group: %s", err) returnErr = true continue } @@ -158,70 +152,61 @@ func (t *TargetGroupSynthesizer) SynthesizeTriggeredTargetGroup(ctx context.Cont tgStatus.TargetGroupID, resTargetGroup.Spec.Config.IsServiceImport, resTargetGroup.Spec.Config.K8SHTTPRouteName) - t.log.Infof("targetGroup Synthesized successfully for %v: %v", resTargetGroup.Spec, tgStatus) + t.log.Debugf("Successfully synthesized target group %s", resTargetGroup.Spec.Name) } } - } - t.log.Infof("Done -- SynthesizeTriggeredTargetGroup %v", resTargetGroups) - if returnErr { return errors.New("LATTICE-RETRY") } else { return nil } - } func (t *TargetGroupSynthesizer) SynthesizeSDKTargetGroups(ctx context.Context) error { var staleSDKTGs []latticemodel.TargetGroup - sdkTGs, err := t.targetGroupManager.List(ctx) + sdkTGs, err := t.targetGroupManager.List(ctx) if err != nil { - t.log.Debugf("SynthesizeSDKTargetGroups: failed to retrieve sdk TGs %v", err) + t.log.Errorf("Error listing target groups: %s", err) return nil } - t.log.Infof("SynthesizeSDKTargetGroups: here is sdkTGs %v len %v", sdkTGs, len(sdkTGs)) - for _, sdkTG := range sdkTGs { tgRouteName := "" if *sdkTG.getTargetGroupOutput.Config.VpcIdentifier != config.VpcID { - t.log.Infof("Ignore target group ARN %v Name %v for other VPCs", + t.log.Debugf("Ignoring target group %s (%s) because it is configured for other VPCs", *sdkTG.getTargetGroupOutput.Arn, *sdkTG.getTargetGroupOutput.Name) continue } // does target group have K8S tags, ignore if it is not tagged tgTags := sdkTG.targetGroupTags - if tgTags == nil || tgTags.Tags == nil { - t.log.Infof("Ignore target group not tagged for K8S, %v, %v", + t.log.Debugf("Ignoring target group %s (%s) because it is not tagged for K8S", *sdkTG.getTargetGroupOutput.Arn, *sdkTG.getTargetGroupOutput.Name) continue } parentRef, ok := tgTags.Tags[latticemodel.K8SParentRefTypeKey] if !ok || parentRef == nil { - t.log.Infof("Ignore target group that have no K8S parentRef tag :%v, %v", + t.log.Debugf("Ignoring target group %s (%s) because it has no K8S parentRef tag", *sdkTG.getTargetGroupOutput.Arn, *sdkTG.getTargetGroupOutput.Name) continue } srvName, ok := tgTags.Tags[latticemodel.K8SServiceNameKey] - if !ok || srvName == nil { - t.log.Infof("Ignore TargetGroup have no servicename tag: %v, %v", + t.log.Debugf("Ignoring target group %s (%s) because it has no servicename tag", *sdkTG.getTargetGroupOutput.Arn, *sdkTG.getTargetGroupOutput.Name) continue } srvNamespace, ok := tgTags.Tags[latticemodel.K8SServiceNamespaceKey] - if !ok || srvNamespace == nil { - t.log.Infof("Ignore TargetGroup have no servicenamespace tag: %v, %v", + t.log.Infof("Ignoring target group %s (%s) because it has no serviceNamespace tag", *sdkTG.getTargetGroupOutput.Arn, *sdkTG.getTargetGroupOutput.Name) continue } @@ -229,20 +214,17 @@ func (t *TargetGroupSynthesizer) SynthesizeSDKTargetGroups(ctx context.Context) // if its parentref is service export, check the parent service export exist // Ignore if service export exists if *parentRef == latticemodel.K8SServiceExportType { - t.log.Infof("TargetGroup %v, %v is referenced by ServiceExport", + t.log.Debugf("TargetGroup %s (%s) is referenced by ServiceExport", *sdkTG.getTargetGroupOutput.Arn, *sdkTG.getTargetGroupOutput.Name) - t.log.Infof("Determine serviceexport name=%v, namespace=%v exists for targetGroup %v", - *srvName, *srvNamespace, *sdkTG.getTargetGroupOutput.Arn) - srvExportName := types.NamespacedName{ Namespace: *srvNamespace, Name: *srvName, } + srvExport := &mcs_api.ServiceExport{} if err := t.client.Get(ctx, srvExportName, srvExport); err == nil { - - t.log.Infof("Ignore TargetGroup(triggered by serviceexport) %v, %v since serviceexport object is found", + t.log.Debugf("Ignoring target group %s (%s), which was triggered by serviceexport, since serviceexport object is found", *sdkTG.getTargetGroupOutput.Arn, *sdkTG.getTargetGroupOutput.Name) continue } @@ -251,22 +233,20 @@ func (t *TargetGroupSynthesizer) SynthesizeSDKTargetGroups(ctx context.Context) // if its parentRef is a route, check that the parent route exists // Ignore if route does not exist if *parentRef == latticemodel.K8SHTTPRouteType { - t.log.Infof("TargetGroup %v, %v is referenced by a route", + t.log.Debugf("Target group %s (%s) is referenced by a route", *sdkTG.getTargetGroupOutput.Arn, *sdkTG.getTargetGroupOutput.Name) routeNameValue, ok := tgTags.Tags[latticemodel.K8SHTTPRouteNameKey] tgRouteName = *routeNameValue - if !ok || routeNameValue == nil { - t.log.Infof("Ignore TargetGroup(triggered by route) %v, %v has no route name tag", + t.log.Infof("Ignoring target group %s (%s), which was triggered by a route, because it has no route name tag", *sdkTG.getTargetGroupOutput.Arn, *sdkTG.getTargetGroupOutput.Name) continue } routeNamespaceValue, ok := tgTags.Tags[latticemodel.K8SHTTPRouteNamespaceKey] - if !ok || routeNamespaceValue == nil { - t.log.Infof("Ignore TargetGroup(triggered by route) %v, %v has no route namespace tag", + t.log.Infof("Ignoring target group %s (%s), which was triggered by a route, because it has no route namespace tag", *sdkTG.getTargetGroupOutput.Arn, *sdkTG.getTargetGroupOutput.Name) continue } @@ -279,11 +259,11 @@ func (t *TargetGroupSynthesizer) SynthesizeSDKTargetGroups(ctx context.Context) var route core.Route if *sdkTG.getTargetGroupOutput.Config.ProtocolVersion == vpclattice.TargetGroupProtocolVersionGrpc { if route, err = core.GetGRPCRoute(ctx, t.client, routeName); err != nil { - t.log.Infof("Could not find GRPCRoute for target group %s", err) + t.log.Errorf("Could not find GRPCRoute for target group %s", err) } } else { if route, err = core.GetHTTPRoute(ctx, t.client, routeName); err != nil { - t.log.Infof("Could not find HTTPRoute for target group %s", err) + t.log.Errorf("Could not find HTTPRoute for target group %s", err) } } @@ -295,22 +275,22 @@ func (t *TargetGroupSynthesizer) SynthesizeSDKTargetGroups(ctx context.Context) isUsed := t.isTargetGroupUsedByRoute(ctx, tgName, route) && len(sdkTG.getTargetGroupOutput.ServiceArns) > 0 if isUsed { - t.log.Infof("Ignore TargetGroup(triggered by route) %v, %v since route object is found", + t.log.Infof("Ignoring target group %s (%s), which was triggered by a route, since route object is found", *sdkTG.getTargetGroupOutput.Arn, *sdkTG.getTargetGroupOutput.Name) continue } else { - t.log.Infof("tgname %v is not used by route %v", tgName, route) + t.log.Infof("target group %s is not used by route %s-%s", tgName, route.Name(), route.Namespace()) } } } // the routename for serviceimport is "" if tg, err := t.latticeDataStore.GetTargetGroup(*sdkTG.getTargetGroupOutput.Name, "", true); err == nil { - t.log.Infof("Ignore target group created by service import %v", tg) + t.log.Debugf("Ignoring target group %s, which was created by service import", tg.TargetGroupKey.Name) continue } - t.log.Debugf("Append stale SDK TG to stale list Name %v, routename %v, ARN %v", + t.log.Debugf("Appending stale target group to stale list. Name: %s, routename: %s, ARN: %s", *sdkTG.getTargetGroupOutput.Name, tgRouteName, *sdkTG.getTargetGroupOutput.Id) staleSDKTGs = append(staleSDKTGs, latticemodel.TargetGroup{ @@ -325,28 +305,22 @@ func (t *TargetGroupSynthesizer) SynthesizeSDKTargetGroups(ctx context.Context) } - t.log.Infof("SynthesizeSDKTargetGroups, here is the stale target groups list %v stalelen %d", staleSDKTGs, len(staleSDKTGs)) - - ret_err := false + retErr := false for _, sdkTG := range staleSDKTGs { - err := t.targetGroupManager.Delete(ctx, &sdkTG) - t.log.Debugf("SynthesizeSDKTargetGroups, deleting stale target group %v", err) - if err != nil && !strings.Contains(err.Error(), "TargetGroup is referenced in routing configuration, listeners or rules of service.") { - ret_err = true + t.log.Debugf("Error deleting target group %s", err) + retErr = true } // continue on even when there is an err - } - if ret_err { + if retErr { return errors.New(LATTICE_RETRY) } else { return nil } - } func (t *TargetGroupSynthesizer) isTargetGroupUsedByRoute(ctx context.Context, tgName string, route core.Route) bool { @@ -378,30 +352,36 @@ func (t *TargetGroupSynthesizer) PostSynthesize(ctx context.Context) error { func (t *TargetGroupSynthesizer) SynthesizeTriggeredTargetGroupsCreation(ctx context.Context) error { var resTargetGroups []*latticemodel.TargetGroup var returnErr = false - t.stack.ListResources(&resTargetGroups) - t.log.Infof("SynthesizeTriggeredTargetGroupsCreation TargetGroups: [%v]", resTargetGroups) + err := t.stack.ListResources(&resTargetGroups) + if err != nil { + return err + } + for _, resTargetGroup := range resTargetGroups { if resTargetGroup.Spec.IsDeleted { - t.log.Infof("In the SynthesizeTriggeredTargetGroupsCreation(), we only handle TG Creation request and skip any deletion request [%v]", resTargetGroup) + t.log.Debugf("Ignoring deletion request for target group %s", resTargetGroup.Spec.Name) continue } + if resTargetGroup.Spec.Config.IsServiceImport { tgStatus, err := t.targetGroupManager.Get(ctx, resTargetGroup) if err != nil { - t.log.Infof("Error on t.targetGroupManager.Get for %v err %v", resTargetGroup, err) + t.log.Debugf("Error getting target group %s due to %s", resTargetGroup.Spec.Name, err) returnErr = true continue } + // for serviceimport, the httproutename is "" t.latticeDataStore.AddTargetGroup(resTargetGroup.Spec.Name, resTargetGroup.Spec.Config.VpcID, tgStatus.TargetGroupARN, tgStatus.TargetGroupID, resTargetGroup.Spec.Config.IsServiceImport, "") - t.log.Infof("targetGroup Synthesized successfully for %s: %v", resTargetGroup.Spec.Name, tgStatus) + + t.log.Debugf("Successfully synthesized target group %s with status %s", resTargetGroup.Spec.Name, tgStatus) } else { // handle TargetGroup creation request that triggered by httproute with backendref k8sService creation or serviceExport creation resTargetGroup.Spec.Config.VpcID = config.VpcID tgStatus, err := t.targetGroupManager.Create(ctx, resTargetGroup) if err != nil { - t.log.Infof("Error on t.targetGroupManager.Create for %v err %v", resTargetGroup, err) + t.log.Debugf("Error creating target group %s due to %s", resTargetGroup.Spec.Name, err) returnErr = true continue } @@ -411,43 +391,42 @@ func (t *TargetGroupSynthesizer) SynthesizeTriggeredTargetGroupsCreation(ctx con resTargetGroup.Spec.Config.VpcID, tgStatus.TargetGroupARN, tgStatus.TargetGroupID, resTargetGroup.Spec.Config.IsServiceImport, resTargetGroup.Spec.Config.K8SHTTPRouteName) - t.log.Infof("targetGroup Synthesized successfully for %v: %v", resTargetGroup.Spec, tgStatus) + + t.log.Debugf("Successfully synthesized target group %s with status %s", resTargetGroup.Spec.Name, tgStatus) } } - t.log.Infof("Done -- SynthesizeTriggeredTargetGroupsCreation %v", resTargetGroups) + if returnErr { return errors.New(LATTICE_RETRY) } else { return nil } - } func (t *TargetGroupSynthesizer) SynthesizeTriggeredTargetGroupsDeletion(ctx context.Context) error { var resTargetGroups []*latticemodel.TargetGroup var returnErr = false - t.stack.ListResources(&resTargetGroups) + err := t.stack.ListResources(&resTargetGroups) + if err != nil { + return err + } for _, resTargetGroup := range resTargetGroups { - t.log.Debugf("SynthesizeTriggeredTargetGroupsDeletion: TargetGroup ==[%v]", *resTargetGroup) - if !resTargetGroup.Spec.IsDeleted { - t.log.Infof("SynthesizeTriggeredTargetGroupsDeletion ignoring target group deletion request for tg: [%v]", resTargetGroup) + t.log.Infof("Ignoring target group %s because it is not deleted", resTargetGroup.Spec.Name) continue } if resTargetGroup.Spec.Config.IsServiceImport { - t.log.Debugf("Deleting service import target group from local datastore %v", resTargetGroup.Spec.LatticeID) + t.log.Debugf("Deleting service import target group from local datastore %s", resTargetGroup.Spec.LatticeID) t.latticeDataStore.DelTargetGroup(resTargetGroup.Spec.Name, resTargetGroup.Spec.Config.K8SHTTPRouteName, resTargetGroup.Spec.Config.IsServiceImport) } else { // For delete TargetGroup request triggered by k8s service, invoke vpc lattice api to delete it, if success, delete the tg in the datastore as well err := t.targetGroupManager.Delete(ctx, resTargetGroup) - t.log.Infof("err := t.targetGroupManager.Delete(ctx, resTargetGroup) err: %v", err) if err == nil { - t.log.Infof("Delete Target Group in SynthesizeTriggeredTargetGroupsDeletion: successfully deleted target group %v", resTargetGroup) t.latticeDataStore.DelTargetGroup(resTargetGroup.Spec.Name, resTargetGroup.Spec.Config.K8SHTTPRouteName, resTargetGroup.Spec.Config.IsServiceImport) } else { - t.log.Infof("Delete Target Group in SynthesizeTriggeredTargetGroupsDeletion: failed to delete target group %v, err %v", resTargetGroup, err) + t.log.Debugf("Error deleting target group %s due to %s", resTargetGroup.Spec.Name, err) returnErr = true } } diff --git a/pkg/gateway/model_build_lattice_service.go b/pkg/gateway/model_build_lattice_service.go index ca88292d..ddd5f80c 100644 --- a/pkg/gateway/model_build_lattice_service.go +++ b/pkg/gateway/model_build_lattice_service.go @@ -93,7 +93,7 @@ func (t *latticeServiceModelBuildTask) buildModel(ctx context.Context) error { err = t.buildTargets(ctx) if err != nil { - t.log.Errorf("failed to build targets due to %s", err) + t.log.Debugf("failed to build targets due to %s", err) } // only build listener when it is NOT delete case diff --git a/pkg/gateway/model_build_listener.go b/pkg/gateway/model_build_listener.go index 4e5c799c..ed6c8291 100644 --- a/pkg/gateway/model_build_listener.go +++ b/pkg/gateway/model_build_listener.go @@ -99,7 +99,7 @@ func (t *latticeServiceModelBuildTask) buildListener(ctx context.Context) error t.latticeService.Spec.CustomerCertARN = certARN } - t.log.Debugf("Building Listener: found matching listner Port %v", port) + t.log.Debugf("Building Listener: found matching listner Port %d", port) if len(t.route.Spec().Rules()) == 0 { return fmt.Errorf("error building listener, there are no rules for route %s-%s", diff --git a/pkg/gateway/model_build_rule.go b/pkg/gateway/model_build_rule.go index d603eda4..204e2a9b 100644 --- a/pkg/gateway/model_build_rule.go +++ b/pkg/gateway/model_build_rule.go @@ -49,7 +49,6 @@ func (t *latticeServiceModelBuildTask) buildRules(ctx context.Context) error { } for _, rule := range t.route.Spec().Rules() { - t.log.Debugf("Parsing http rule spec: %+v", rule) var ruleSpec latticemodel.RuleSpec if len(rule.Matches()) > 1 { @@ -142,8 +141,7 @@ func (t *latticeServiceModelBuildTask) buildRules(ctx context.Context) error { ruleSpec.NumOfHeaderMatches = len(match.Headers()) - t.log.Debugf("Examining match.Headers %v for route %s-%s", - match.Headers(), t.route.Name(), t.route.Namespace()) + t.log.Debugf("Examining match.Headers for route %s-%s", t.route.Name(), t.route.Namespace()) for i, header := range match.Headers() { t.log.Debugf("Examining match.Header: i = %d header.Type %s", i, *header.Type()) diff --git a/pkg/gateway/model_build_targetgroup.go b/pkg/gateway/model_build_targetgroup.go index 7d483a6d..399cb544 100644 --- a/pkg/gateway/model_build_targetgroup.go +++ b/pkg/gateway/model_build_targetgroup.go @@ -115,7 +115,7 @@ func (t *targetGroupModelBuildTask) buildModel(ctx context.Context) error { err = t.BuildTargets(ctx) if err != nil { - t.log.Errorf("Failed to build targets for service export %s-%s due to %w", + t.log.Debugf("Failed to build targets for service export %s-%s due to %s", t.serviceExport.Name, t.serviceExport.Namespace, err) } @@ -318,7 +318,6 @@ func (t *latticeServiceModelBuildTask) buildTargetGroup( } tg := latticemodel.NewTargetGroup(t.stack, tgName, tgSpec) - t.log.Infof("buildTargetGroup, tg[%s], tgSpec%v \n", tgName, tg) t.tgByResID[tgName] = tg } } From 833839a92fa0e5658f9e4e02c651ba991f3716f0 Mon Sep 17 00:00:00 2001 From: Shawn Kaplan Date: Tue, 3 Oct 2023 11:56:05 -0700 Subject: [PATCH 2/6] Added AccessLogPolicy CRD --- ...-networking.k8s.aws_accesslogpolicies.yaml | 93 +++++++++++++++++++ config/crds/kustomization.yaml | 1 + config/rbac/cluster-role-controller.yaml | 18 ++++ ...-networking.k8s.aws_accesslogpolicies.yaml | 93 +++++++++++++++++++ helm/templates/cluster-role-controller.yaml | 18 ++++ .../v1alpha1/accesslogpolicy_types.go | 65 +++++++++++++ .../v1alpha1/zz_generated.deepcopy.go | 83 +++++++++++++++++ .../v1alpha1/zz_generated.register.go | 2 + 8 files changed, 373 insertions(+) create mode 100644 config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml create mode 100644 helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml create mode 100644 pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go diff --git a/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml b/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml new file mode 100644 index 00000000..c9bc5f95 --- /dev/null +++ b/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml @@ -0,0 +1,93 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.13.0 + name: accesslogpolicies.application-networking.k8s.aws +spec: + group: application-networking.k8s.aws + names: + categories: + - gateway-api + kind: AccessLogPolicy + listKind: AccessLogPolicyList + plural: accesslogpolicies + shortNames: + - tgp + singular: accesslogpolicy + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: AccessLogPolicySpec defines the desired state of AccessLogPolicy. + properties: + protocol: + description: "The Amazon Resource Name (ARN) of the destination that + will store access logs. Supported values are S3 Bucket, CloudWatch + Log Group, and Firehose Delivery Stream ARNs. \n Changes to this + value results in replacement of the VPC Lattice Access Log Subscription." + type: string + targetRef: + description: "TargetRef points to the kubernetes Service or Gateway + resource that will have this policy attached. \n This field is following + the guidelines of Kubernetes Gateway API policy attachment." + properties: + group: + description: Group is the group of the target resource. + maxLength: 253 + pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + kind: + description: Kind is kind of the target resource. + maxLength: 63 + minLength: 1 + pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ + type: string + name: + description: Name is the name of the target resource. + maxLength: 253 + minLength: 1 + type: string + namespace: + description: Namespace is the namespace of the referent. When + unspecified, the local namespace is inferred. Even when policy + targets a resource in a different namespace, it MUST only apply + to traffic originating from the same namespace as the policy. + maxLength: 63 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string + required: + - group + - kind + - name + type: object + required: + - targetRef + type: object + required: + - spec + type: object + served: true + storage: true + subresources: {} diff --git a/config/crds/kustomization.yaml b/config/crds/kustomization.yaml index 3065722c..f1d66224 100644 --- a/config/crds/kustomization.yaml +++ b/config/crds/kustomization.yaml @@ -7,3 +7,4 @@ resources: - bases/externaldns.k8s.io_dnsendpoints.yaml - bases/application-networking.k8s.aws_targetgrouppolicies.yaml - bases/application-networking.k8s.aws_vpcassociationpolicies.yaml + - bases/application-networking.k8s.aws_accesslogpolicies.yaml diff --git a/config/rbac/cluster-role-controller.yaml b/config/rbac/cluster-role-controller.yaml index e728b747..d81834c4 100644 --- a/config/rbac/cluster-role-controller.yaml +++ b/config/rbac/cluster-role-controller.yaml @@ -310,5 +310,23 @@ rules: - application-networking.k8s.aws resources: - vpcassociationpolicies/finalizers + verbs: + - update +- apiGroups: + - application-networking.k8s.aws + resources: + - accesslogpolicies + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - application-networking.k8s.aws + resources: + - accesslogpolicies/finalizers verbs: - update \ No newline at end of file diff --git a/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml b/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml new file mode 100644 index 00000000..c9bc5f95 --- /dev/null +++ b/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml @@ -0,0 +1,93 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.13.0 + name: accesslogpolicies.application-networking.k8s.aws +spec: + group: application-networking.k8s.aws + names: + categories: + - gateway-api + kind: AccessLogPolicy + listKind: AccessLogPolicyList + plural: accesslogpolicies + shortNames: + - tgp + singular: accesslogpolicy + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: AccessLogPolicySpec defines the desired state of AccessLogPolicy. + properties: + protocol: + description: "The Amazon Resource Name (ARN) of the destination that + will store access logs. Supported values are S3 Bucket, CloudWatch + Log Group, and Firehose Delivery Stream ARNs. \n Changes to this + value results in replacement of the VPC Lattice Access Log Subscription." + type: string + targetRef: + description: "TargetRef points to the kubernetes Service or Gateway + resource that will have this policy attached. \n This field is following + the guidelines of Kubernetes Gateway API policy attachment." + properties: + group: + description: Group is the group of the target resource. + maxLength: 253 + pattern: ^$|^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + kind: + description: Kind is kind of the target resource. + maxLength: 63 + minLength: 1 + pattern: ^[a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?$ + type: string + name: + description: Name is the name of the target resource. + maxLength: 253 + minLength: 1 + type: string + namespace: + description: Namespace is the namespace of the referent. When + unspecified, the local namespace is inferred. Even when policy + targets a resource in a different namespace, it MUST only apply + to traffic originating from the same namespace as the policy. + maxLength: 63 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string + required: + - group + - kind + - name + type: object + required: + - targetRef + type: object + required: + - spec + type: object + served: true + storage: true + subresources: {} diff --git a/helm/templates/cluster-role-controller.yaml b/helm/templates/cluster-role-controller.yaml index 695f28bb..8336d0e0 100644 --- a/helm/templates/cluster-role-controller.yaml +++ b/helm/templates/cluster-role-controller.yaml @@ -327,3 +327,21 @@ rules: - vpcassociationpolicies/finalizers verbs: - update +- apiGroups: + - application-networking.k8s.aws + resources: + - accesslogpolicies + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - application-networking.k8s.aws + resources: + - accesslogpolicies/finalizers + verbs: + - update \ No newline at end of file diff --git a/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go b/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go new file mode 100644 index 00000000..69e9d553 --- /dev/null +++ b/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go @@ -0,0 +1,65 @@ +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/aws/aws-application-networking-k8s/pkg/k8s" + "github.com/aws/aws-application-networking-k8s/pkg/model/core" +) + +const ( + AccessLogPolicyKind = "AccessLogPolicy" +) + +// +genclient +// +kubebuilder:object:root=true +// +kubebuilder:resource:categories=gateway-api,shortName=tgp +// +kubebuilder:storageversion +// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` +type AccessLogPolicy struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec AccessLogPolicySpec `json:"spec"` +} + +// +kubebuilder:object:root=true +// AccessLogPolicyList contains a list of AccessLogPolicies. +type AccessLogPolicyList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []AccessLogPolicy `json:"items"` +} + +// AccessLogPolicySpec defines the desired state of AccessLogPolicy. +type AccessLogPolicySpec struct { + // The Amazon Resource Name (ARN) of the destination that will store access logs. + // Supported values are S3 Bucket, CloudWatch Log Group, and Firehose Delivery Stream ARNs. + // + // Changes to this value results in replacement of the VPC Lattice Access Log Subscription. + // +optional + DestinationArn *string `json:"protocol,omitempty"` + + // TargetRef points to the kubernetes Service or Gateway resource that will have this policy attached. + // + // This field is following the guidelines of Kubernetes Gateway API policy attachment. + TargetRef *v1alpha2.PolicyTargetReference `json:"targetRef"` +} + +func (p *AccessLogPolicy) GetTargetRef() *v1alpha2.PolicyTargetReference { + return p.Spec.TargetRef +} + +func (p *AccessLogPolicy) GetNamespacedName() types.NamespacedName { + return k8s.NamespacedName(p) +} + +func (pl *AccessLogPolicyList) GetItems() []core.Policy { + items := make([]core.Policy, len(pl.Items)) + for i, item := range pl.Items { + items[i] = &item + } + return items +} diff --git a/pkg/apis/applicationnetworking/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/applicationnetworking/v1alpha1/zz_generated.deepcopy.go index 9d83c8d0..58db11de 100644 --- a/pkg/apis/applicationnetworking/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/applicationnetworking/v1alpha1/zz_generated.deepcopy.go @@ -9,6 +9,89 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessLogPolicy) DeepCopyInto(out *AccessLogPolicy) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessLogPolicy. +func (in *AccessLogPolicy) DeepCopy() *AccessLogPolicy { + if in == nil { + return nil + } + out := new(AccessLogPolicy) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *AccessLogPolicy) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessLogPolicyList) DeepCopyInto(out *AccessLogPolicyList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]AccessLogPolicy, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessLogPolicyList. +func (in *AccessLogPolicyList) DeepCopy() *AccessLogPolicyList { + if in == nil { + return nil + } + out := new(AccessLogPolicyList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *AccessLogPolicyList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessLogPolicySpec) DeepCopyInto(out *AccessLogPolicySpec) { + *out = *in + if in.DestinationArn != nil { + in, out := &in.DestinationArn, &out.DestinationArn + *out = new(string) + **out = **in + } + if in.TargetRef != nil { + in, out := &in.TargetRef, &out.TargetRef + *out = new(v1alpha2.PolicyTargetReference) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessLogPolicySpec. +func (in *AccessLogPolicySpec) DeepCopy() *AccessLogPolicySpec { + if in == nil { + return nil + } + out := new(AccessLogPolicySpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HealthCheckConfig) DeepCopyInto(out *HealthCheckConfig) { *out = *in diff --git a/pkg/apis/applicationnetworking/v1alpha1/zz_generated.register.go b/pkg/apis/applicationnetworking/v1alpha1/zz_generated.register.go index 3ee000e2..f1f91b55 100644 --- a/pkg/apis/applicationnetworking/v1alpha1/zz_generated.register.go +++ b/pkg/apis/applicationnetworking/v1alpha1/zz_generated.register.go @@ -57,6 +57,8 @@ func init() { // Adds the list of known types to Scheme. func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, + &AccessLogPolicy{}, + &AccessLogPolicyList{}, &TargetGroupPolicy{}, &TargetGroupPolicyList{}, &VpcAssociationPolicy{}, From f322ba44c1a94ff0b015fca5cd814de02a011593 Mon Sep 17 00:00:00 2001 From: Shawn Kaplan Date: Tue, 3 Oct 2023 15:22:51 -0700 Subject: [PATCH 3/6] Fixed alp shortname and destinationArn json, plus added kubebuilder validation --- .../application-networking.k8s.aws_accesslogpolicies.yaml | 2 +- .../applicationnetworking/v1alpha1/accesslogpolicy_types.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml b/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml index c9bc5f95..5a40c469 100644 --- a/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml +++ b/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml @@ -14,7 +14,7 @@ spec: listKind: AccessLogPolicyList plural: accesslogpolicies shortNames: - - tgp + - alp singular: accesslogpolicy scope: Namespaced versions: diff --git a/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go b/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go index 69e9d553..bbb1dc94 100644 --- a/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go +++ b/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go @@ -15,7 +15,7 @@ const ( // +genclient // +kubebuilder:object:root=true -// +kubebuilder:resource:categories=gateway-api,shortName=tgp +// +kubebuilder:resource:categories=gateway-api,shortName=alp // +kubebuilder:storageversion // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` type AccessLogPolicy struct { @@ -40,7 +40,8 @@ type AccessLogPolicySpec struct { // // Changes to this value results in replacement of the VPC Lattice Access Log Subscription. // +optional - DestinationArn *string `json:"protocol,omitempty"` + // +kubebuilder:validation:Pattern=`^arn(:[a-z0-9]+([.-][a-z0-9]+)*){2}(:([a-z0-9]+([.-][a-z0-9]+)*)?){2}:([^/].*)?` + DestinationArn *string `json:"destinationArn,omitempty"` // TargetRef points to the kubernetes Service or Gateway resource that will have this policy attached. // From a26ce021375101117df039f826960eab47f3bea6 Mon Sep 17 00:00:00 2001 From: Shawn Kaplan Date: Tue, 3 Oct 2023 15:32:42 -0700 Subject: [PATCH 4/6] Added missing yaml changes --- .../application-networking.k8s.aws_accesslogpolicies.yaml | 3 ++- .../application-networking.k8s.aws_accesslogpolicies.yaml | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml b/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml index 5a40c469..cc3abc37 100644 --- a/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml +++ b/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml @@ -41,11 +41,12 @@ spec: spec: description: AccessLogPolicySpec defines the desired state of AccessLogPolicy. properties: - protocol: + destinationArn: description: "The Amazon Resource Name (ARN) of the destination that will store access logs. Supported values are S3 Bucket, CloudWatch Log Group, and Firehose Delivery Stream ARNs. \n Changes to this value results in replacement of the VPC Lattice Access Log Subscription." + pattern: ^arn(:[a-z0-9]+([.-][a-z0-9]+)*){2}(:([a-z0-9]+([.-][a-z0-9]+)*)?){2}:([^/].*)? type: string targetRef: description: "TargetRef points to the kubernetes Service or Gateway diff --git a/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml b/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml index c9bc5f95..cc3abc37 100644 --- a/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml +++ b/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml @@ -14,7 +14,7 @@ spec: listKind: AccessLogPolicyList plural: accesslogpolicies shortNames: - - tgp + - alp singular: accesslogpolicy scope: Namespaced versions: @@ -41,11 +41,12 @@ spec: spec: description: AccessLogPolicySpec defines the desired state of AccessLogPolicy. properties: - protocol: + destinationArn: description: "The Amazon Resource Name (ARN) of the destination that will store access logs. Supported values are S3 Bucket, CloudWatch Log Group, and Firehose Delivery Stream ARNs. \n Changes to this value results in replacement of the VPC Lattice Access Log Subscription." + pattern: ^arn(:[a-z0-9]+([.-][a-z0-9]+)*){2}(:([a-z0-9]+([.-][a-z0-9]+)*)?){2}:([^/].*)? type: string targetRef: description: "TargetRef points to the kubernetes Service or Gateway From 6e0b988af941ea16c2fd4770987daf83c1e7cf95 Mon Sep 17 00:00:00 2001 From: Shawn Kaplan Date: Wed, 4 Oct 2023 09:20:25 -0700 Subject: [PATCH 5/6] Fixed documentation for support access log policy target ref kinds --- .../application-networking.k8s.aws_accesslogpolicies.yaml | 7 ++++--- .../application-networking.k8s.aws_accesslogpolicies.yaml | 7 ++++--- .../v1alpha1/accesslogpolicy_types.go | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml b/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml index cc3abc37..cd3831ea 100644 --- a/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml +++ b/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml @@ -49,9 +49,10 @@ spec: pattern: ^arn(:[a-z0-9]+([.-][a-z0-9]+)*){2}(:([a-z0-9]+([.-][a-z0-9]+)*)?){2}:([^/].*)? type: string targetRef: - description: "TargetRef points to the kubernetes Service or Gateway - resource that will have this policy attached. \n This field is following - the guidelines of Kubernetes Gateway API policy attachment." + description: "TargetRef points to the Kubernetes Gateway, HTTPRoute, + or GRPCRoute resource that will have this policy attached. \n This + field is following the guidelines of Kubernetes Gateway API policy + attachment." properties: group: description: Group is the group of the target resource. diff --git a/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml b/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml index cc3abc37..cd3831ea 100644 --- a/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml +++ b/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml @@ -49,9 +49,10 @@ spec: pattern: ^arn(:[a-z0-9]+([.-][a-z0-9]+)*){2}(:([a-z0-9]+([.-][a-z0-9]+)*)?){2}:([^/].*)? type: string targetRef: - description: "TargetRef points to the kubernetes Service or Gateway - resource that will have this policy attached. \n This field is following - the guidelines of Kubernetes Gateway API policy attachment." + description: "TargetRef points to the Kubernetes Gateway, HTTPRoute, + or GRPCRoute resource that will have this policy attached. \n This + field is following the guidelines of Kubernetes Gateway API policy + attachment." properties: group: description: Group is the group of the target resource. diff --git a/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go b/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go index bbb1dc94..78312fb2 100644 --- a/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go +++ b/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go @@ -43,7 +43,7 @@ type AccessLogPolicySpec struct { // +kubebuilder:validation:Pattern=`^arn(:[a-z0-9]+([.-][a-z0-9]+)*){2}(:([a-z0-9]+([.-][a-z0-9]+)*)?){2}:([^/].*)?` DestinationArn *string `json:"destinationArn,omitempty"` - // TargetRef points to the kubernetes Service or Gateway resource that will have this policy attached. + // TargetRef points to the Kubernetes Gateway, HTTPRoute, or GRPCRoute resource that will have this policy attached. // // This field is following the guidelines of Kubernetes Gateway API policy attachment. TargetRef *v1alpha2.PolicyTargetReference `json:"targetRef"` From 969edac9bfecf66023d4a445d185235dbde13274 Mon Sep 17 00:00:00 2001 From: Shawn Kaplan Date: Wed, 4 Oct 2023 12:24:53 -0700 Subject: [PATCH 6/6] Added status to Access Log Policy --- ...-networking.k8s.aws_accesslogpolicies.yaml | 103 +++++++++++++++++- config/rbac/cluster-role-controller.yaml | 10 +- ...-networking.k8s.aws_accesslogpolicies.yaml | 103 +++++++++++++++++- .../v1alpha1/accesslogpolicy_types.go | 28 +++++ .../v1alpha1/zz_generated.deepcopy.go | 24 ++++ 5 files changed, 265 insertions(+), 3 deletions(-) diff --git a/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml b/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml index cd3831ea..75f57f25 100644 --- a/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml +++ b/config/crds/bases/application-networking.k8s.aws_accesslogpolicies.yaml @@ -87,9 +87,110 @@ spec: required: - targetRef type: object + status: + default: + conditions: + - lastTransitionTime: "1970-01-01T00:00:00Z" + message: Waiting for controller + reason: NotReconciled + status: Unknown + type: Accepted + description: Status defines the current state of AccessLogPolicy. + properties: + conditions: + default: + - lastTransitionTime: "1970-01-01T00:00:00Z" + message: Waiting for controller + reason: Pending + status: Unknown + type: Accepted + - lastTransitionTime: "1970-01-01T00:00:00Z" + message: Waiting for controller + reason: Pending + status: Unknown + type: Programmed + description: "Conditions describe the current conditions of the AccessLogPolicy. + \n Implementations should prefer to express Policy conditions using + the `PolicyConditionType` and `PolicyConditionReason` constants + so that operators and tools can converge on a common vocabulary + to describe AccessLogPolicy state. \n Known condition types are: + \n * \"Accepted\" * \"Ready\"" + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + \n type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: \"Available\", + \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge + // +listType=map // +listMapKey=type Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should be when + the underlying condition changed. If that is not known, then + using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, if .metadata.generation + is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current + state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier indicating + the reason for the condition's last transition. Producers + of specific condition types may define expected values and + meanings for this field, and whether the values are considered + a guaranteed API. The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + maxItems: 8 + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + type: object required: - spec type: object served: true storage: true - subresources: {} + subresources: + status: {} diff --git a/config/rbac/cluster-role-controller.yaml b/config/rbac/cluster-role-controller.yaml index d81834c4..f25eb4cf 100644 --- a/config/rbac/cluster-role-controller.yaml +++ b/config/rbac/cluster-role-controller.yaml @@ -329,4 +329,12 @@ rules: resources: - accesslogpolicies/finalizers verbs: - - update \ No newline at end of file + - update +- apiGroups: + - application-networking.k8s.aws + resources: + - accesslogpolicies/status + verbs: + - get + - patch + - update diff --git a/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml b/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml index cd3831ea..75f57f25 100644 --- a/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml +++ b/helm/crds/application-networking.k8s.aws_accesslogpolicies.yaml @@ -87,9 +87,110 @@ spec: required: - targetRef type: object + status: + default: + conditions: + - lastTransitionTime: "1970-01-01T00:00:00Z" + message: Waiting for controller + reason: NotReconciled + status: Unknown + type: Accepted + description: Status defines the current state of AccessLogPolicy. + properties: + conditions: + default: + - lastTransitionTime: "1970-01-01T00:00:00Z" + message: Waiting for controller + reason: Pending + status: Unknown + type: Accepted + - lastTransitionTime: "1970-01-01T00:00:00Z" + message: Waiting for controller + reason: Pending + status: Unknown + type: Programmed + description: "Conditions describe the current conditions of the AccessLogPolicy. + \n Implementations should prefer to express Policy conditions using + the `PolicyConditionType` and `PolicyConditionReason` constants + so that operators and tools can converge on a common vocabulary + to describe AccessLogPolicy state. \n Known condition types are: + \n * \"Accepted\" * \"Ready\"" + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + \n type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: \"Available\", + \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge + // +listType=map // +listMapKey=type Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should be when + the underlying condition changed. If that is not known, then + using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, if .metadata.generation + is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current + state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier indicating + the reason for the condition's last transition. Producers + of specific condition types may define expected values and + meanings for this field, and whether the values are considered + a guaranteed API. The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + maxItems: 8 + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + type: object required: - spec type: object served: true storage: true - subresources: {} + subresources: + status: {} diff --git a/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go b/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go index 78312fb2..5fd7fa2a 100644 --- a/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go +++ b/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go @@ -18,11 +18,17 @@ const ( // +kubebuilder:resource:categories=gateway-api,shortName=alp // +kubebuilder:storageversion // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` +// +kubebuilder:subresource:status type AccessLogPolicy struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` Spec AccessLogPolicySpec `json:"spec"` + + // Status defines the current state of AccessLogPolicy. + // + // +kubebuilder:default={conditions: {{type: "Accepted", status: "Unknown", reason:"NotReconciled", message:"Waiting for controller", lastTransitionTime: "1970-01-01T00:00:00Z"}}} + Status AccessLogPolicyStatus `json:"status,omitempty"` } // +kubebuilder:object:root=true @@ -49,6 +55,28 @@ type AccessLogPolicySpec struct { TargetRef *v1alpha2.PolicyTargetReference `json:"targetRef"` } +// AccessLogPolicyStatus defines the observed state of AccessLogPolicy. +type AccessLogPolicyStatus struct { + // Conditions describe the current conditions of the AccessLogPolicy. + // + // Implementations should prefer to express Policy conditions + // using the `PolicyConditionType` and `PolicyConditionReason` + // constants so that operators and tools can converge on a common + // vocabulary to describe AccessLogPolicy state. + // + // Known condition types are: + // + // * "Accepted" + // * "Ready" + // + // +optional + // +listType=map + // +listMapKey=type + // +kubebuilder:validation:MaxItems=8 + // +kubebuilder:default={{type: "Accepted", status: "Unknown", reason:"Pending", message:"Waiting for controller", lastTransitionTime: "1970-01-01T00:00:00Z"},{type: "Programmed", status: "Unknown", reason:"Pending", message:"Waiting for controller", lastTransitionTime: "1970-01-01T00:00:00Z"}} + Conditions []metav1.Condition `json:"conditions,omitempty"` +} + func (p *AccessLogPolicy) GetTargetRef() *v1alpha2.PolicyTargetReference { return p.Spec.TargetRef } diff --git a/pkg/apis/applicationnetworking/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/applicationnetworking/v1alpha1/zz_generated.deepcopy.go index 58db11de..d09d8f91 100644 --- a/pkg/apis/applicationnetworking/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/applicationnetworking/v1alpha1/zz_generated.deepcopy.go @@ -5,6 +5,7 @@ package v1alpha1 import ( + "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -15,6 +16,7 @@ func (in *AccessLogPolicy) DeepCopyInto(out *AccessLogPolicy) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessLogPolicy. @@ -92,6 +94,28 @@ func (in *AccessLogPolicySpec) DeepCopy() *AccessLogPolicySpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AccessLogPolicyStatus) DeepCopyInto(out *AccessLogPolicyStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AccessLogPolicyStatus. +func (in *AccessLogPolicyStatus) DeepCopy() *AccessLogPolicyStatus { + if in == nil { + return nil + } + out := new(AccessLogPolicyStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HealthCheckConfig) DeepCopyInto(out *HealthCheckConfig) { *out = *in