Add DNS attributes to VPC spec
#38
Conversation
| return nil | ||
| } | ||
|
|
||
| func (rm *resourceManager) createAttributes( |
There was a problem hiding this comment.
Currently this method makes two modify calls, can this be optimized to make single AWS API call?
There was a problem hiding this comment.
Agree with @vijtrip2 here (though I'm confused why Vijay is reviewing code during his PTO...).
There was a problem hiding this comment.
I share the concern over api calls (now and long-term), but I think if we consolidate the calls the api returns an error:
Both properties are set using the ModifyVpcAttribute method, but can only be updated one at a time.
There was a problem hiding this comment.
I'm so glad you asked!
ModifyVpcAttribute can only modify ONE attribute at a time. DescribeVpcAttribute can only describe ONE attribute at a time. Really great stuff.
| @@ -48,6 +48,12 @@ spec: | |||
| its canonical form; for example, if you specify 100.68.0.18/18, | |||
| we modify it to 100.68.0.0/18. | |||
| type: string | |||
| enableDNSHostnames: | |||
| description: The attribute value. The valid values are true or false. | |||
There was a problem hiding this comment.
LOL. Great description :)
There was a problem hiding this comment.
Creating an issue for custom documentation rn
There was a problem hiding this comment.
:) I didn't expect you to do that.
There was a problem hiding this comment.
It's been really bugging me. Especially that now we are creating resource references, they should have proper documentation - rather than being the GoDoc for the ACKResourceReference type
| return nil | ||
| } | ||
|
|
||
| func (rm *resourceManager) createAttributes( |
There was a problem hiding this comment.
Agree with @vijtrip2 here (though I'm confused why Vijay is reviewing code during his PTO...).
RedbackThomson
force-pushed
the
vpc-attributes
branch
from
43f37ec to
a548cb1
Compare
|
/test ec2-kind-e2e |
|
[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 |
Issue #, if available: aws-controllers-k8s/community#1215
Description of changes:
Adds two new fields to the
VPCspec:EnableDNSSupportandEnableDNSHostnames. Both properties are set using theModifyVpcAttributemethod, but can only be updated one at a time.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.