From 8befc334a8aa8ad0cfa170e463ea9408ed9fdffc Mon Sep 17 00:00:00 2001 From: shuqz Date: Fri, 1 May 2026 17:48:43 -0700 Subject: [PATCH] [feat i2g]add feature flag for ingress plan annotation --- controllers/ingress/dryrun.go | 33 +++++++ controllers/ingress/dryrun_test.go | 96 +++++++++++++++++++ controllers/ingress/group_controller.go | 8 ++ docs/deploy/configurations.md | 1 + helm/aws-load-balancer-controller/values.yaml | 1 + pkg/annotations/constants.go | 1 + pkg/config/feature_gates.go | 2 + pkg/gateway/constants/controller_constants.go | 6 ++ pkg/ingress2gateway/translate/translate.go | 48 ++++++++++ pkg/ingress2gateway/utils/constants.go | 5 + 10 files changed, 201 insertions(+) create mode 100644 controllers/ingress/dryrun.go create mode 100644 controllers/ingress/dryrun_test.go diff --git a/controllers/ingress/dryrun.go b/controllers/ingress/dryrun.go new file mode 100644 index 000000000..e58b0eda3 --- /dev/null +++ b/controllers/ingress/dryrun.go @@ -0,0 +1,33 @@ +package ingress + +import ( + "context" + "fmt" + + networking "k8s.io/api/networking/v1" + "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" + "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + dryRunPlanAnnotation = annotations.AnnotationPrefixIngress + "/" + annotations.IngressSuffixDryRunPlan +) + +// patchDryRunPlanAnnotation writes the serialized stack JSON to the dry-run-plan +// annotation on the given ingress. It skips the patch if the value is unchanged +// to avoid unnecessary API calls and reconcile loops. +func patchDryRunPlanAnnotation(ctx context.Context, k8sClient client.Client, ing *networking.Ingress, planJSON string) error { + if ing.Annotations[dryRunPlanAnnotation] == planJSON { + return nil + } + ingOld := ing.DeepCopy() + if ing.Annotations == nil { + ing.Annotations = map[string]string{} + } + ing.Annotations[dryRunPlanAnnotation] = planJSON + if err := k8sClient.Patch(ctx, ing, client.MergeFrom(ingOld)); err != nil { + return fmt.Errorf("failed to patch dry-run plan annotation on ingress %s: %w", k8s.NamespacedName(ing), err) + } + return nil +} diff --git a/controllers/ingress/dryrun_test.go b/controllers/ingress/dryrun_test.go new file mode 100644 index 000000000..d9cac0861 --- /dev/null +++ b/controllers/ingress/dryrun_test.go @@ -0,0 +1,96 @@ +package ingress + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + networking "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func Test_patchDryRunPlanAnnotation(t *testing.T) { + tests := []struct { + name string + ingAnnotations map[string]string + planJSON string + wantAnnotation string + wantOtherAnnotations map[string]string + }{ + { + name: "writes annotation when not present", + ingAnnotations: nil, + planJSON: `{"id":"test-stack"}`, + wantAnnotation: `{"id":"test-stack"}`, + }, + { + name: "skips patch when value unchanged", + ingAnnotations: map[string]string{ + dryRunPlanAnnotation: `{"id":"test-stack"}`, + }, + planJSON: `{"id":"test-stack"}`, + wantAnnotation: `{"id":"test-stack"}`, + }, + { + name: "updates annotation when value changed", + ingAnnotations: map[string]string{ + dryRunPlanAnnotation: `{"id":"old-stack"}`, + }, + planJSON: `{"id":"new-stack"}`, + wantAnnotation: `{"id":"new-stack"}`, + }, + { + name: "preserves existing annotations", + ingAnnotations: map[string]string{ + "alb.ingress.kubernetes.io/scheme": "internet-facing", + }, + planJSON: `{"id":"test-stack"}`, + wantAnnotation: `{"id":"test-stack"}`, + wantOtherAnnotations: map[string]string{ + "alb.ingress.kubernetes.io/scheme": "internet-facing", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ing := &networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress", + Namespace: "default", + Annotations: tt.ingAnnotations, + }, + } + + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(ing). + Build() + + err := patchDryRunPlanAnnotation(context.Background(), k8sClient, ing, tt.planJSON) + require.NoError(t, err) + + // Verify the annotation was persisted + updatedIng := &networking.Ingress{} + err = k8sClient.Get(context.Background(), types.NamespacedName{ + Name: "test-ingress", + Namespace: "default", + }, updatedIng) + require.NoError(t, err) + assert.Equal(t, tt.wantAnnotation, updatedIng.Annotations[dryRunPlanAnnotation]) + + // Verify other annotations were not clobbered + for k, v := range tt.wantOtherAnnotations { + assert.Equal(t, v, updatedIng.Annotations[k], "annotation %s should be preserved", k) + } + }) + } +} diff --git a/controllers/ingress/group_controller.go b/controllers/ingress/group_controller.go index e3b70297a..753f8b217 100644 --- a/controllers/ingress/group_controller.go +++ b/controllers/ingress/group_controller.go @@ -92,6 +92,7 @@ func NewGroupReconciler(cloud services.Cloud, k8sClient client.Client, eventReco groupLoader: groupLoader, groupFinalizerManager: groupFinalizerManager, + featureGates: controllerConfig.FeatureGates, logger: logger, metricsCollector: metricsCollector, controllerName: controllerName, @@ -114,6 +115,7 @@ type groupReconciler struct { groupLoader ingress.GroupLoader groupFinalizerManager ingress.FinalizerManager + featureGates config.FeatureGates logger logr.Logger metricsCollector lbcmetrics.MetricCollector controllerName string @@ -230,6 +232,12 @@ func (r *groupReconciler) buildAndDeployModel(ctx context.Context, ingGroup ingr } r.logger.Info("successfully built model", "model", stackJSON) + if r.featureGates.Enabled(config.IngressPlanAnnotation) && len(ingGroup.Members) > 0 { + if err := patchDryRunPlanAnnotation(ctx, r.k8sClient, ingGroup.Members[0].Ing, stackJSON); err != nil { + r.logger.Error(err, "failed to patch dry-run plan annotation", "ingress", k8s.NamespacedName(ingGroup.Members[0].Ing)) + } + } + deployModelFn := func() { err = r.stackDeployer.Deploy(ctx, stack, r.metricsCollector, "ingress") } diff --git a/docs/deploy/configurations.md b/docs/deploy/configurations.md index 2f1574225..9393d95ca 100644 --- a/docs/deploy/configurations.md +++ b/docs/deploy/configurations.md @@ -201,3 +201,4 @@ There are a set of key=value pairs that describe AWS load balancer controller fe | GatewayListenerSet | string | true | Enable or disable the usage of ListenerSets in the Gateway API | | ALBTargetControlAgent | string | false | Enable or disable the ALB Target Control Agent | | EnableCertificateManagement | string | false | Whether to enable the [Certificate Management feature](../guide/ingress/certificate_management.md). | +| IngressPlanAnnotation | string | false | If enabled, the controller writes the serialized model stack JSON to the `alb.ingress.kubernetes.io/dry-run-plan` annotation on ingress. For grouped ingresses, the annotation is written to the first member (lowest group order). | diff --git a/helm/aws-load-balancer-controller/values.yaml b/helm/aws-load-balancer-controller/values.yaml index 0b852bc3c..4383a97f2 100644 --- a/helm/aws-load-balancer-controller/values.yaml +++ b/helm/aws-load-balancer-controller/values.yaml @@ -428,6 +428,7 @@ controllerConfig: # ALBGatewayAPI: true # GatewayListenerSet: true # GlobalAcceleratorController: false + # IngressPlanAnnotation: false # EnhancedDefaultBehavior: false # EnableDefaultTagsLowPriority: false # ALBTargetControlAgent: false diff --git a/pkg/annotations/constants.go b/pkg/annotations/constants.go index 76ce7e96f..b116b0bfc 100644 --- a/pkg/annotations/constants.go +++ b/pkg/annotations/constants.go @@ -81,6 +81,7 @@ const ( IngressSuffixTargetControlPort = "target-control-port" IngressSuffixCreateCertificate = "create-acm-cert" IngressSuffixACMCaARN = "acm-pca-arn" + IngressSuffixDryRunPlan = "dry-run-plan" // NLB annotation suffixes // prefixes service.beta.kubernetes.io, service.kubernetes.io diff --git a/pkg/config/feature_gates.go b/pkg/config/feature_gates.go index 8060bebac..1fbe5d15b 100644 --- a/pkg/config/feature_gates.go +++ b/pkg/config/feature_gates.go @@ -40,6 +40,7 @@ const ( ALBTargetControlAgent Feature = "ALBTargetControlAgent" GatewayListenerSet Feature = "GatewayListenerSet" EnableCertificateManagement Feature = "EnableCertificateManagement" + IngressPlanAnnotation Feature = "IngressPlanAnnotation" ) type FeatureGates interface { @@ -94,6 +95,7 @@ func NewFeatureGates() FeatureGates { ALBTargetControlAgent: generateDefaultFeatureStatus(false), GatewayListenerSet: generateDefaultFeatureStatus(true), EnableCertificateManagement: generateDefaultFeatureStatus(false), + IngressPlanAnnotation: generateDefaultFeatureStatus(false), }, } } diff --git a/pkg/gateway/constants/controller_constants.go b/pkg/gateway/constants/controller_constants.go index 8d28af01c..ec5c8f47f 100644 --- a/pkg/gateway/constants/controller_constants.go +++ b/pkg/gateway/constants/controller_constants.go @@ -102,4 +102,10 @@ const ( // AnnotationDryRunEnabledValue is the value that enables dry-run mode on a Gateway. AnnotationDryRunEnabledValue = "true" + + // AnnotationIngressPlanHolder is the annotation on a Gateway that points to the + // namespace/name of the ingress holding the dry-run-plan for an explicit ingress group. + // Set by the migration tool so the in-cluster console can locate the ingress model. + // it is only needed for group ingress + AnnotationIngressPlanHolder = "gateway.k8s.aws/ingress-plan-holder" ) diff --git a/pkg/ingress2gateway/translate/translate.go b/pkg/ingress2gateway/translate/translate.go index efc3cb436..c62fad3e4 100644 --- a/pkg/ingress2gateway/translate/translate.go +++ b/pkg/ingress2gateway/translate/translate.go @@ -3,6 +3,8 @@ package translate import ( "fmt" "os" + "sort" + "strconv" "strings" corev1 "k8s.io/api/core/v1" @@ -11,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1" gatewayv1beta1 "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1" + annotations "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" gwconstants "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/constants" "sigs.k8s.io/aws-load-balancer-controller/pkg/ingress2gateway" "sigs.k8s.io/aws-load-balancer-controller/pkg/ingress2gateway/utils" @@ -107,6 +110,18 @@ func Translate(in *ingress2gateway.InputResources) (*ingress2gateway.OutputResou crossNSGroupName = group.name } gw := buildGateway(gatewayName, group.namespace, lbConfig, allPorts, crossNSGroupName) + + // For explicit groups, find the member holding the dry-run plan and write + // a pointer annotation on the Gateway so the in-cluster console can locate it. + if group.isExplicit { + if holder := findPlanHolder(group.members); holder != "" { + if gw.Annotations == nil { + gw.Annotations = map[string]string{} + } + gw.Annotations[utils.IngressPlanHolderAnnotation] = holder + } + } + out.Gateways = append(out.Gateways, gw) // --- SSL redirect route --- @@ -240,3 +255,36 @@ func buildIngressClassParamsMap(classes []networking.IngressClass, params []elbv } return result } + +// findPlanHolder returns the namespace/name of the group member that the ingress +// controller writes the dry-run-plan annotation to. This is Members[0] after sorting +// by group.order (default 0), with ties broken by lexical namespace/name — the same +// logic used by the ingress controller's group loader. +func findPlanHolder(members []networking.Ingress) string { + sorted := make([]networking.Ingress, len(members)) + copy(sorted, members) + sort.SliceStable(sorted, func(i, j int) bool { + orderI := getGroupOrder(sorted[i]) + orderJ := getGroupOrder(sorted[j]) + if orderI != orderJ { + return orderI < orderJ + } + nameI := fmt.Sprintf("%s/%s", sorted[i].Namespace, sorted[i].Name) + nameJ := fmt.Sprintf("%s/%s", sorted[j].Namespace, sorted[j].Name) + return nameI < nameJ + }) + return fmt.Sprintf("%s/%s", sorted[0].Namespace, sorted[0].Name) +} + +// getGroupOrder parses the group.order annotation from an ingress, defaulting to 0. +func getGroupOrder(ing networking.Ingress) int32 { + val, ok := ing.Annotations[ingressAnnotationKeyPrefix+annotations.IngressSuffixGroupOrder] + if !ok { + return 0 + } + order, err := strconv.ParseInt(val, 10, 32) + if err != nil { + return 0 + } + return int32(order) +} diff --git a/pkg/ingress2gateway/utils/constants.go b/pkg/ingress2gateway/utils/constants.go index c1289bf3f..b03d0a37c 100644 --- a/pkg/ingress2gateway/utils/constants.go +++ b/pkg/ingress2gateway/utils/constants.go @@ -16,6 +16,11 @@ const ( // MigrationTagKey is the AWS tag key used to track migration source. MigrationTagKey = "gateway.k8s.aws/migrated-from" + // IngressPlanHolderAnnotation is the annotation key on a Gateway that points to the + // namespace/name of the ingress holding the dry-run-plan for an explicit ingress group. + // For gateway migrated from single ingress, we read AWS tag `migrated-from` for this purpose + IngressPlanHolderAnnotation = "gateway.k8s.aws/ingress-plan-holder" + // ProtocolHTTP is the HTTP protocol string used in listen-ports and ProtocolPort. ProtocolHTTP = "HTTP"