Support security groups with cyclic references #213
Conversation
helm/values.yaml
Outdated
| featureGates: {} | ||
| # featureGate1: true | ||
| # featureGate2: false | ||
| featureGates: | ||
| CARMv2: false |
There was a problem hiding this comment.
I think we need #214 before merging this
| ec2_validator.assert_security_group(resource_id_1, exists=False) | ||
| ec2_validator.assert_security_group(resource_id_2, exists=False) | ||
| ec2_validator.assert_security_group(resource_id_3, exists=False) |
There was a problem hiding this comment.
Can we also assert the the kubernetes resources are marked as ACKSynced=true?
There was a problem hiding this comment.
you highlighted the section after the resources are being deleted, does the comment still apply?
when the resources are created the assertion exists
There was a problem hiding this comment.
Ohhh correct, my eyes were looking for this call https://github.com/aws-controllers-k8s/test-infra/blob/main/src/acktest/k8s/condition.py#L69 - but your appraoch is correct as well
| sgCpy := r.ko.DeepCopy() | ||
| sgCpy.Spec.IngressRules = nil | ||
| sgCpy.Spec.EgressRules = nil | ||
| if err := rm.syncSGRules(ctx, &resource{ko: sgCpy}, r); err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
Do we need to detach the rules before SG deletion?
There was a problem hiding this comment.
Yes, otherwise we constantly run into a circular DependencyViolation error.
TiberiuGC
force-pushed
the
improvement/security-groups-with-cyclic-refs
branch
from
3c79f32
to
2aac5ea
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: a-hilaly, TiberiuGC 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#2119 Description of changes: Cyclic references support is done via. the following workflow: 1. skip runtime reference state validations by setting `SecurityGroup.Rules.UserIDGroupPairs.GroupID.skip_resource_state_validations: true` (see aws-controllers-k8s/code-generator#544). This allows runtime to proceed with the `sdkCreate` call. 2. inside `sdkCreate` and `sdkUpdate` add custom logic that checks whether referenced security groups are being created on AWS end (i.e. `groupID != nil`). If the checks succeed, move forward with syncing SG rules. Otherwise, requeue and wait for all referenced SGs to be created. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Issue #, if available: aws-controllers-k8s/community#2119
Description of changes:
Cyclic references support is done via. the following workflow:
SecurityGroup.Rules.UserIDGroupPairs.GroupID.skip_resource_state_validations: true(see Allow skipping state validations for resources that support cyclic references code-generator#544). This allows runtime to proceed with thesdkCreatecall.sdkCreateandsdkUpdateadd custom logic that checks whether referenced security groups are being created on AWS end (i.e.groupID != nil). If the checks succeed, move forward with syncing SG rules. Otherwise, requeue and wait for all referenced SGs to be created.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.