- Notifications
You must be signed in to change notification settings - Fork 67
Support Standalone Service Creation & Surface Service ARN #818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments
| return false | ||
| } | ||
| | ||
| // Trim whitespace to be more forgiving of user input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's combine this with previous if?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I will address this in the next revision.
pkg/k8s/utils.go Outdated
| } | ||
| | ||
| // Convert to lowercase for case-insensitive comparison | ||
| switch strings.ToLower(trimmed) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we thinking for a different set of values in the future? If not why do we need a switch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, this is overkill now. I'll simplify it.
docs/faq.md Outdated
| | ||
| 1. **AWS RAM sharing** to share the service with other accounts/VPCs | ||
| 2. **VPC peering or Transit Gateway** for network connectivity | ||
| 3. **Manual service network association** in target VPCs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- AWS RAM sharing to share the service with other accounts to associate with service networks
2.remove VPC peering or Transit Gateway for networking connectivity. Standalone Lattice services resolve to 169.254.171.0/24, which are not routable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed this wording in the latest revision.
| The controller will: | ||
| 1. Remove existing service network associations | ||
| 2. Keep the VPC Lattice service running | ||
| 3. Update the route status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will have to make sure that the service is removed only from the Gateway that is set to stand-alone, as the same service could belong to other Gateways that are not standlone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added detailed steps on how to mitigate this and correctly share with RAM manually.
docs/guides/standalone-services.md Outdated
| ## Limitations | ||
| | ||
| - Standalone services are not discoverable through service network DNS resolution | ||
| - Service network policies do not apply to standalone services |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qualify that service network policies do not apply to standalone services only in the Gateways/service networks they are disconnected from. If the same service is also in a different Gateway/service network, the policies on those Gateways/service networks apply.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I clarified this in the latest revision.
docs/guides/standalone-services.md Outdated
| | ||
| - Standalone services are not discoverable through service network DNS resolution | ||
| - Service network policies do not apply to standalone services | ||
| - Cross-VPC communication requires explicit service sharing or alternative connectivity No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requires explicit service sharing and/or association with service networks.
Remove "alternate connectivity" as there is no other way to connect than through a service network
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated this wording.
| value: /networked | ||
| backendRefs: | ||
| - name: networked-service | ||
| port: 80 No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an example of httpsroute and TLSroute would be helpful with the TLS certs and how they can be added as well the same way as a connected service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added additional examples now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What type of PR is this?
Feature
Which issue does this PR fix:
#691
What does this PR do / Why do we need it:
This PR implements standalone VPC Lattice services that are not automatically associated with service networks. Currently, the AWS Application Networking Controller only processes routes that are always associated with a service network. This enhancement allows for decoupling of service creation and ownership from service network membership, providing more flexibility in service management scenarios.
Key changes:
application-networking.k8s.aws/standalone: "true") to control service network association behaviorIf an issue # is not available please add repro steps and logs from aws-gateway-controller showing the issue:
See issue #691 for detailed requirements and use case description. The issue requests the ability to create VPC Lattice services without automatic service network association to support scenarios where service creation and service network membership need to be managed independently.
Testing done on this change:
Automation added to e2e:
Yes - Integration tests have been added covering:
Will this PR introduce any new dependencies?:
No new dependencies. The feature uses existing Kubernetes annotations and AWS VPC Lattice APIs that are already in use by the controller.
Will this break upgrades or downgrades. Has updating a running cluster been tested?:
No breaking changes. The feature is fully backward compatible:
Does this PR introduce any user-facing change?:
Yes - This introduces new annotation-based configuration options for users.
Do all end-to-end tests successfully pass when running
make e2e-test?:Yes - All existing e2e tests pass, and new integration tests for standalone functionality have been added and are passing.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.