-
Notifications
You must be signed in to change notification settings - Fork 53
Add route table assocations to Subnet
#45
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
Add route table assocations to Subnet
#45
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.
@RedbackThomson great work on this. You made the user experience of associating route tables to subnets entirely painless and aligned with the k8s resource model.
Couple teeny things to address inline but this is very close.
@@ -897,6 +897,7 @@ type Explanation struct { | |||
ExplanationCode *string `json:"explanationCode,omitempty"` | |||
MissingComponent *string `json:"missingComponent,omitempty"` | |||
PacketField *string `json:"packetField,omitempty"` | |||
Protocols []*string `json:"protocols,omitempty"` |
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.
Interesting.. did this come in from an updated aws-sdk-go version or something?
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.
No clue why this showed up, honestly. I haven't touched the SDK version afaik
e141ab7
to
98d2d4d
Compare
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.
This is great and a really clean implementation. Appreciate the clear comments and cx (per jay's comment). Just had some small nits
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brycahta, RedbackThomson The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Closes aws-controllers-k8s/community#1219
Description of changes:
Generates the
Subnet
CRD with a new custom fieldRouteTables
, which represent an array of route table IDs. Route tables are associated with theSubnet
using theAssociateRouteTable
API call, and then updated by deleting the association (usingDisassociateRouteTable
) and re-associating.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.