Skip to content

Conversation

@dlutsch
Copy link
Contributor

@dlutsch dlutsch commented Dec 5, 2025

What type of PR is this?

bug

Which issue does this PR fix:

N/A - Bug discovered while testing the ServiceNetworkOverrideMode feature introduced in recent releases.

What does this PR do / Why do we need it:

Fixes a critical bug where the ServiceNetworkOverrideMode feature is non-functional because the helm chart is missing the environment variable mapping.

The controller code supports ENABLE_SERVICE_NETWORK_OVERRIDE, and pkg/config/controller_config.go parses it from the environment, but helm/templates/deployment.yaml doesn't include the mapping from helm value to environment variable. This makes the feature completely unusable via helm deployments.

Impact: Any deployment using enableServiceNetworkOverride: true in helm values will silently fail - the controller never receives the environment variable and override mode never activates.

If an issue # is not available please add repro steps and logs from aws-gateway-controller showing the issue:

Reproduction Steps

  1. Deploy controller with helm chart:
helm install gateway-api-controller ... --set enableServiceNetworkOverride=true
  1. Verify environment variable is NOT set in pod:
kubectl get deployment ... -o jsonpath='{.spec.template.spec.containers[0].env[*].name}'
# Output: REGION, AWS_ACCOUNT_ID, CLUSTER_VPC_ID, ..., DEFAULT_SERVICE_NETWORK, LOG_LEVEL
# Missing: ENABLE_SERVICE_NETWORK_OVERRIDE
  1. Create Gateway with name different from service network
  2. Gateway fails with "VPC Lattice Service Network not found"
  3. Controller searches for network matching Gateway name instead of using DEFAULT_SERVICE_NETWORK

Log Evidence

Controller logs show it's using Gateway name as network name (proof override mode isn't active):

{"level":"info","msg":"Creating service with service network association ... (networks: [my-gateway])"}

Expected with override mode: (networks: [sn-xxxxxxxxxxxxx]) (the configured DEFAULT_SERVICE_NETWORK)

Testing done on this change:

Tested in EKS cluster

  1. Applied terraform with enableServiceNetworkOverride = "true" helm value
  2. Without fix: Environment variable not in pod, override mode inactive
  3. Applied fix: Built custom image with fixed helm chart
  4. After fix: Environment variable present, controller uses DEFAULT_SERVICE_NETWORK

Automation added to e2e:

No - This is a helm chart configuration bug, not a functional feature addition. Existing ServiceNetworkOverride tests cover the feature itself.

Will this PR introduce any new dependencies?:

No - This only adds a missing helm template line.

Will this break upgrades or downgrades. Has updating a running cluster been tested?:

  • Upgrades: Safe - adds missing environment variable
  • Downgrades: Safe - removing env var just disables override mode (existing behavior)
  • Tested: Yes, tested upgrade in sandbox cluster with successful pod restart

Does this PR introduce any user-facing change?:

Yes - makes ServiceNetworkOverrideMode feature actually work via helm.

Fixes broken ServiceNetworkOverrideMode helm configuration. Users can now enable service network override mode by setting `enableServiceNetworkOverride: true` in helm values.

Do all end-to-end tests successfully pass when running make e2e-test?:

Not run for this PR - this is a 2-line helm template fix with no logic changes. The ServiceNetworkOverrideMode feature itself already has test coverage in pkg/gateway/model_build_lattice_service_test.go.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Dan Lutsch and others added 2 commits December 5, 2025 13:07
The helm chart was missing the environment variable mapping for
enableServiceNetworkOverride, which prevented the controller from
enabling service network override mode even when the value was
configured via terraform.
@SinghVikram97 SinghVikram97 added this pull request to the merge queue Dec 6, 2025
Merged via the queue into aws:main with commit f667f41 Dec 6, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants