Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
47736fa
fix(VPCPeeringConnection): rm omit_unchanged_field
marcdavoli Dec 7, 2023
180b277
fix: add code to reconcile peeringOptions fields
marcdavoli Dec 7, 2023
b0f8238
test: add peeringOptions assertions
marcdavoli Dec 7, 2023
447b7dc
fix: typo in test status path
marcdavoli Dec 7, 2023
ada138c
test: add test_update_peering_options
marcdavoli Dec 7, 2023
6d17e81
test: remove all python tests, temporarily
marcdavoli Dec 7, 2023
edcbf16
test: fix boolean values
marcdavoli Dec 7, 2023
3531194
test: ko instead of res.ko
marcdavoli Dec 8, 2023
bb7ae0a
chore: add fields to update payload
marcdavoli Dec 8, 2023
38b46f5
chore: add logging
marcdavoli Dec 8, 2023
74c7f03
chore: if delta.DifferentExcept from false to true
marcdavoli Dec 8, 2023
d132e08
chore: fix logging
marcdavoli Dec 8, 2023
6e24e35
test: see if acceptRequest is blocking peeringOpti
marcdavoli Dec 8, 2023
ca33e7b
fix: test assert status code
marcdavoli Dec 8, 2023
0871455
fix: test assert status code
marcdavoli Dec 8, 2023
85e151d
chore: more logging
marcdavoli Dec 8, 2023
c66870c
fix: minor logic flow corrections
marcdavoli Dec 8, 2023
6d596bd
test: remove deprecated fields
marcdavoli Dec 8, 2023
944d3aa
chore: remove debug logging
marcdavoli Dec 8, 2023
013d6af
fix: ref test
marcdavoli Dec 8, 2023
735055c
test: get cr again after requeue
marcdavoli Dec 8, 2023
e89f614
test: reorder asserts
marcdavoli Dec 8, 2023
3afb3f5
docs: add relevant comments to tests
marcdavoli Dec 8, 2023
5961085
test: remove asserts on peeringOptions
marcdavoli Dec 10, 2023
b2f56cb
test: set peeringoptions to true
marcdavoli Dec 10, 2023
579848e
chore: keep track of test file
marcdavoli Dec 11, 2023
bd2cd19
chore: keep track of test file
marcdavoli Dec 11, 2023
8d7c8d6
revert: test file to main
marcdavoli Dec 11, 2023
ce38866
revert: test file to main
marcdavoli Dec 11, 2023
bb6f50c
revert: test file to main
marcdavoli Dec 11, 2023
a84f03a
test: flip ENABLE_DNS_HOSTNAMES to True
marcdavoli Dec 11, 2023
5b2c50d
test: remove peeringOptions from simple_vpc
marcdavoli Dec 11, 2023
3c56676
test: readd omit_unchanged_fields
marcdavoli Dec 11, 2023
93e3747
test: rename vpc
marcdavoli Dec 11, 2023
2e99b66
fix: test failing because of misnamed VPC
marcdavoli Dec 11, 2023
015a285
revert: vpc name
marcdavoli Dec 11, 2023
c3865d8
revert: add all tests back
marcdavoli Dec 11, 2023
8c28eb5
chore: remove omit_unchanged_fields
marcdavoli Dec 11, 2023
a6068cd
test: improve robustness of Tags test
marcdavoli Dec 11, 2023
a7bb876
fix: check the right resource for latest status
marcdavoli Dec 12, 2023
1b50ed0
Merge branch 'main' into fix(VPCPeeringConnection)/peeringOptions-not…
marcdavoli Dec 12, 2023
abb07dc
test: add test_update_peering_options
marcdavoli Dec 12, 2023
78eeaaf
fix: boto3 syntax
marcdavoli Dec 12, 2023
57158a7
test: remove all test to go faster
marcdavoli Dec 12, 2023
2f16cf3
fix: list entry
marcdavoli Dec 12, 2023
de61ee2
fix: use the right values to begin with
marcdavoli Dec 12, 2023
3410131
revert: add peeringoptions to simple instead
marcdavoli Dec 12, 2023
7b04fbd
chore: separate out peering options test
marcdavoli Dec 12, 2023
32cb8eb
fix: add @pytest.fixture
marcdavoli Dec 13, 2023
8dde2e8
chore: rm boto3 and use helper func instead
marcdavoli Dec 13, 2023
e6acaab
test: print vpc_cr
marcdavoli Dec 13, 2023
bea960c
chore: use ref test instead of sharedvpc
marcdavoli Dec 13, 2023
ad5b4fb
chore: new test fixture, to set bools to false
marcdavoli Dec 13, 2023
68942ee
chore: actually use the new test fixture
marcdavoli Dec 13, 2023
dfe4918
test: add peeringoptions wait condition
marcdavoli Dec 13, 2023
8f8bd9d
test: wait longer
marcdavoli Dec 13, 2023
e05b575
chore: actually use the new test manifest
marcdavoli Dec 13, 2023
ab67513
test: add fallback to boto3
marcdavoli Dec 13, 2023
d135721
test: add fallback to boto3
marcdavoli Dec 13, 2023
1c3d165
fix: case Dns > DNS
marcdavoli Dec 13, 2023
3e0fbf1
fix: convert bool to str
marcdavoli Dec 13, 2023
d176998
test: remove print statements
marcdavoli Dec 13, 2023
5a15b2d
docs: add comments to defs
marcdavoli Dec 13, 2023
5b339dc
chore: lint python code
marcdavoli Dec 13, 2023
d8d28a8
test: add all of them back
marcdavoli Dec 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions apis/v1alpha1/ack-generate-metadata.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
ack_generate_info:
build_date: "2023-12-06T21:43:43Z"
build_hash: 892f29d00a4c4ad21a2fa32919921de18190979d
go_version: go1.21.1
version: v0.27.1
api_directory_checksum: 6e2d850d97f2f72db31c9bef522eca4ab95b3fcd
build_date: "2023-12-12T12:34:35Z"
build_hash: 3653329ceeb20015851b8776a6061a3fb0ec2935
go_version: go1.21.0
version: "3653329"
api_directory_checksum: d452bf19bfd1496aacdc215bf7cc9ea86c55c122
api_version: v1alpha1
aws_sdk_go_version: v1.44.93
generator_config_info:
file_checksum: d10a62517f87bacf988184f8c454b90b42dee732
file_checksum: df1057de840147701c11acfdbed0e28e9b0ddd96
original_file_name: generator.yaml
last_modification:
reason: API generation
2 changes: 0 additions & 2 deletions apis/v1alpha1/generator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -818,8 +818,6 @@ resources:
path: Tags
compare:
is_ignored: True
update_operation:
omit_unchanged_fields: true
Comment on lines -821 to -822
Copy link
Contributor Author

@marcdavoli marcdavoli Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this, because I tested the behavior with & without it, and got many problem with it turned on:

  • Turning peeringOptions to True on after the resource was created with False did not update the resource
  • Creating a new resource with peeringOptions to True would revert the field to False for some fields and update the resource properly for others

It was agreed with Amine H. that we will remove this for the time being and potentially add it back later as an optimization once we've worked out the kinks for this specific use case of the flag.

hooks:
delta_pre_compare:
code: compareTags(delta, a, b)
Expand Down
2 changes: 0 additions & 2 deletions generator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -818,8 +818,6 @@ resources:
path: Tags
compare:
is_ignored: True
update_operation:
omit_unchanged_fields: true
hooks:
delta_pre_compare:
code: compareTags(delta, a, b)
Expand Down
6 changes: 3 additions & 3 deletions pkg/resource/vpc_peering_connection/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ import (

var (
ErrVPCPeeringConnectionCreating = fmt.Errorf(
"VPCPerringConnection in '%v' state, cannot be modified or deleted",
"VPCPeeringConnection in '%v' state, cannot be modified or deleted",
"creating",
)
ErrVPCPeeringConnectionProvisioning = fmt.Errorf(
"VPCPerringConnection in '%v' state, cannot be modified or deleted",
"VPCPeeringConnection in '%v' state, cannot be modified or deleted",
svcsdk.VpcPeeringConnectionStateReasonCodeProvisioning,
)
ErrVPCPeeringConnectionDeleting = fmt.Errorf(
"VPCPerringConnection in '%v' state, cannot be modified or deleted",
"VPCPeeringConnection in '%v' state, cannot be modified or deleted",
svcsdk.VpcPeeringConnectionStateReasonCodeDeleting,
)
)
Expand Down
131 changes: 94 additions & 37 deletions pkg/resource/vpc_peering_connection/sdk.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,10 +1,42 @@


if r.ko.Spec.AccepterPeeringConnectionOptions != nil {
f0 := &svcapitypes.PeeringConnectionOptionsRequest{}
if r.ko.Spec.AccepterPeeringConnectionOptions.AllowDNSResolutionFromRemoteVPC != nil {
f0.AllowEgressFromLocalClassicLinkToRemoteVPC = r.ko.Spec.AccepterPeeringConnectionOptions.AllowDNSResolutionFromRemoteVPC
}
if r.ko.Spec.AccepterPeeringConnectionOptions.AllowEgressFromLocalClassicLinkToRemoteVPC != nil {
f0.AllowEgressFromLocalClassicLinkToRemoteVPC = r.ko.Spec.AccepterPeeringConnectionOptions.AllowEgressFromLocalClassicLinkToRemoteVPC
}
if r.ko.Spec.AccepterPeeringConnectionOptions.AllowEgressFromLocalVPCToRemoteClassicLink != nil {
f0.AllowEgressFromLocalVPCToRemoteClassicLink = r.ko.Spec.AccepterPeeringConnectionOptions.AllowEgressFromLocalVPCToRemoteClassicLink
}
ko.Spec.AccepterPeeringConnectionOptions = f0
} else {
ko.Spec.AccepterPeeringConnectionOptions = nil
}
if r.ko.Spec.RequesterPeeringConnectionOptions != nil {
f1 := &svcapitypes.PeeringConnectionOptionsRequest{}
if r.ko.Spec.RequesterPeeringConnectionOptions.AllowDNSResolutionFromRemoteVPC != nil {
f1.AllowDNSResolutionFromRemoteVPC = r.ko.Spec.RequesterPeeringConnectionOptions.AllowDNSResolutionFromRemoteVPC
}
if r.ko.Spec.RequesterPeeringConnectionOptions.AllowEgressFromLocalClassicLinkToRemoteVPC != nil {
f1.AllowEgressFromLocalClassicLinkToRemoteVPC = r.ko.Spec.RequesterPeeringConnectionOptions.AllowEgressFromLocalClassicLinkToRemoteVPC
}
if r.ko.Spec.RequesterPeeringConnectionOptions.AllowEgressFromLocalVPCToRemoteClassicLink != nil {
f1.AllowEgressFromLocalVPCToRemoteClassicLink = r.ko.Spec.RequesterPeeringConnectionOptions.AllowEgressFromLocalVPCToRemoteClassicLink
}
ko.Spec.RequesterPeeringConnectionOptions = f1
} else {
ko.Spec.RequesterPeeringConnectionOptions = nil
}

// Artificially trigger detection by delta.DifferentAt("Spec.AcceptRequest")
res := &resource{ko}
if isVPCPeeringConnectionPendingAcceptance(res) {
res.ko.Spec.AcceptRequest = aws.Bool(false)
} else if isVPCPeeringConnectionActive(res) || isVPCPeeringConnectionProvisioning(res) {
res.ko.Spec.AcceptRequest = aws.Bool(true)
} else if isVPCPeeringConnectionCreating(res) {
return nil, requeueWaitWhileCreating
}
return res, requeueWaitWhileCreating
}
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@

if isVPCPeeringConnectionCreating(desired) {
if isVPCPeeringConnectionCreating(latest) {
Copy link
Contributor Author

@marcdavoli marcdavoli Dec 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using desired would intermittently cause the resource to go in an infinite loop during creation if the initial status of the Peering resource was Initializing (Creating) and even if the AWS-side resource was updated, this line would still throw error messages like:

"error": "VPCPerringConnection in 'creating' state, cannot be modified or deleted"}

return desired, requeueWaitWhileCreating
}
if isVPCPeeringConnectionProvisioning(desired) {
if isVPCPeeringConnectionProvisioning(latest) {
return desired, requeueWaitWhileProvisioning
}
if isVPCPeeringConnectionDeleting(desired) {
if isVPCPeeringConnectionDeleting(latest) {
return desired, requeueWaitWhileDeleting
}

// in case of pending acceptance or accepted state we make the updates.
// If the VPC Peering Connection is Pending Acceptance or Active, continue

if delta.DifferentAt("Spec.Tags") {
if err := rm.syncTags(ctx, desired, latest); err != nil {
return nil, err
// This causes a requeue and the rest of the fields will be synced on the next reconciliation loop
ackcondition.SetSynced(desired, corev1.ConditionFalse, nil, nil)
return desired, err
}
}

Expand Down Expand Up @@ -44,8 +46,38 @@
}
}

// Only continue if something other than Tags or certain fields has changed in the Spec
if !delta.DifferentExcept("Spec.Tags", "Spec.AcceptRequest") {
return desired, nil
}

// Only continue if something other than Tags or certain fields has changed in the Spec
if !delta.DifferentExcept("Spec.Tags", "Spec.AcceptRequest") {
return desired, nil
}
if desired.ko.Spec.AccepterPeeringConnectionOptions != nil {
f0 := &svcapitypes.PeeringConnectionOptionsRequest{}
if desired.ko.Spec.AccepterPeeringConnectionOptions.AllowDNSResolutionFromRemoteVPC != nil {
f0.AllowDNSResolutionFromRemoteVPC = desired.ko.Spec.AccepterPeeringConnectionOptions.AllowDNSResolutionFromRemoteVPC
}
if desired.ko.Spec.AccepterPeeringConnectionOptions.AllowEgressFromLocalClassicLinkToRemoteVPC != nil {
f0.AllowEgressFromLocalClassicLinkToRemoteVPC = desired.ko.Spec.AccepterPeeringConnectionOptions.AllowEgressFromLocalClassicLinkToRemoteVPC
}
if desired.ko.Spec.AccepterPeeringConnectionOptions.AllowEgressFromLocalVPCToRemoteClassicLink != nil {
f0.AllowEgressFromLocalVPCToRemoteClassicLink = desired.ko.Spec.AccepterPeeringConnectionOptions.AllowEgressFromLocalVPCToRemoteClassicLink
}
desired.ko.Spec.AccepterPeeringConnectionOptions = f0
} else {
desired.ko.Spec.AccepterPeeringConnectionOptions = nil
}
if desired.ko.Spec.RequesterPeeringConnectionOptions != nil {
f1 := &svcapitypes.PeeringConnectionOptionsRequest{}
if desired.ko.Spec.RequesterPeeringConnectionOptions.AllowDNSResolutionFromRemoteVPC != nil {
f1.AllowDNSResolutionFromRemoteVPC = desired.ko.Spec.RequesterPeeringConnectionOptions.AllowDNSResolutionFromRemoteVPC
}
if desired.ko.Spec.RequesterPeeringConnectionOptions.AllowEgressFromLocalClassicLinkToRemoteVPC != nil {
f1.AllowEgressFromLocalClassicLinkToRemoteVPC = desired.ko.Spec.RequesterPeeringConnectionOptions.AllowEgressFromLocalClassicLinkToRemoteVPC
}
if desired.ko.Spec.RequesterPeeringConnectionOptions.AllowEgressFromLocalVPCToRemoteClassicLink != nil {
f1.AllowEgressFromLocalVPCToRemoteClassicLink = desired.ko.Spec.RequesterPeeringConnectionOptions.AllowEgressFromLocalVPCToRemoteClassicLink
}
desired.ko.Spec.RequesterPeeringConnectionOptions = f1
} else {
desired.ko.Spec.RequesterPeeringConnectionOptions = nil
}
8 changes: 0 additions & 8 deletions test/e2e/resources/vpc_peering_connection.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,6 @@ spec:
vpcID: $VPC_ID
peerVPCID: $PEER_VPC_ID
acceptRequest: true
requesterPeeringConnectionOptions:
allowDNSResolutionFromRemoteVPC: true
allowEgressFromLocalClassicLinkToRemoteVPC: true
allowEgressFromLocalVPCToRemoteClassicLink: true
accepterPeeringConnectionOptions:
allowDNSResolutionFromRemoteVPC: true
allowEgressFromLocalClassicLinkToRemoteVPC: true
allowEgressFromLocalVPCToRemoteClassicLink: true
Comment on lines -9 to -16
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "simple" test only tests creation and Acceptance of the VPC Peering Connection.

tags:
- key: $TAG_KEY
value: $TAG_VALUE
12 changes: 12 additions & 0 deletions test/e2e/resources/vpc_peering_connection_peering_options.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apiVersion: ec2.services.k8s.aws/v1alpha1
kind: VPCPeeringConnection
metadata:
name: $VPC_PEERING_CONNECTION_NAME
spec:
vpcID: $VPC_ID
peerVPCID: $PEER_VPC_ID
acceptRequest: true
requesterPeeringConnectionOptions:
allowDNSResolutionFromRemoteVPC: true
accepterPeeringConnectionOptions:
allowDNSResolutionFromRemoteVPC: true
Loading