Skip to content
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.

Sign up for GitHub

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

fix(ec2): fix ipv6 field issues in ec2 group #109

Merged
merged 3 commits into from
Jun 8, 2023
+69 −4
Merged

fix(ec2): fix ipv6 field issues in ec2 group #109

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
53c7f77
fix(ec2): fix ipv6 field issues in ec2 group
Apr 28, 2023
9c3a47d
fix(ec2): fix ipv6 field issues in ec2 group
Apr 28, 2023
c1ec336
Include ipv6_addresses to IgnoredFields
turkenf Jun 7, 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
3 changes: 3 additions & 0 deletions apis/ec2/v1beta1/zz_generated_terraformed.go
View file
Open in desktop

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

4 changes: 3 additions & 1 deletion config/ec2/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ func Configure(p *config.Provider) {
"source_dest_check",
"vpc_security_group_ids",
"associate_public_ip_address",
"ipv6_addresses",
"ipv6_address_count",
},
}
config.MoveToStatus(r.TerraformResource, "security_groups")
Expand Down Expand Up @@ -173,7 +175,7 @@ func Configure(p *config.Provider) {
}
r.LateInitializer = config.LateInitializer{
IgnoredFields: []string{
"interface_type", "private_ip_list", "private_ips",
"interface_type", "private_ip_list", "private_ips", "ipv6_address_count",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"interface_type", "private_ip_list", "private_ips", "ipv6_address_count",
"interface_type", "private_ip_list", "private_ips", ipv6_addresses, "ipv6_address_count",

I think we usually include both conflicting fields in the IgnoredFields slice.

@ulucinar @sergenyalcin please keep me honest here.

Copy link
Collaborator

@ulucinar ulucinar May 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, agreed. When the native schema declares two configuration arguments as mutually exclusive, we need to ignore them both during late-initialization because we may have set either of them under spec.forProvider and the unset one could be late-initialized at runtime causing a conflict. The argument ipv6_addresses is declared to conflict with the ipv6_address_count and similarly, ipv6_address_count is declared to conflict with with ipv6_addresses.

In this specific case, the relationship is mutual, i.e., ipv6_addresses & ipv6_address_count are mutually exclusive as dictated in the native (Terraform) schema. In theory, this may not always be true (consider a case in which if argument a is set then b gets initialized from a but the reverse is not true, i.e., if b is set, a does not get initialized). I don't know any practical examples of this and I think we had better assume a mutual relationship for late-initialization.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Included ipv6_addresses to IgnoredFields.

},
}
// Mutually exclusive with aws_network_interface_attachment
Expand Down
65 changes: 62 additions & 3 deletions examples/ec2/instance.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ kind: Instance
metadata:
annotations:
uptest.upbound.io/timeout: "3600"
meta.upbound.io/example-id: ec2/v1beta1/instance
labels:
testing.upbound.io/example-name: sample-instance
name: sample-instance
spec:
forProvider:
Expand All @@ -11,7 +14,63 @@ spec:
instanceType: t2.micro
networkInterface:
- deviceIndex: 0
networkInterfaceIdRef:
name: sample-ni
networkInterfaceIdSelector:
matchLabels:
testing.upbound.io/example-name: sample-instance
creditSpecification:
- cpuCredits: unlimited
- cpuCredits: unlimited

---

apiVersion: ec2.aws.upbound.io/v1beta1
kind: NetworkInterface
metadata:
annotations:
meta.upbound.io/example-id: ec2/v1beta1/instance
labels:
testing.upbound.io/example-name: sample-instance
name: sample-instance
spec:
forProvider:
region: us-west-1
subnetIdSelector:
matchLabels:
testing.upbound.io/example-name: sample-instance
privateIps:
- "172.16.10.100"

---

apiVersion: ec2.aws.upbound.io/v1beta1
kind: Subnet
metadata:
annotations:
meta.upbound.io/example-id: ec2/v1beta1/instance
labels:
testing.upbound.io/example-name: sample-instance
name: sample-instance
spec:
forProvider:
region: us-west-1
availabilityZone: us-west-1b
vpcIdSelector:
matchLabels:
testing.upbound.io/example-name: sample-instance
cidrBlock: 172.16.10.0/24

---

apiVersion: ec2.aws.upbound.io/v1beta1
kind: VPC
metadata:
annotations:
meta.upbound.io/example-id: ec2/v1beta1/instance
labels:
testing.upbound.io/example-name: sample-instance
name: sample-instance
spec:
forProvider:
region: us-west-1
cidrBlock: 172.16.0.0/16
tags:
Name: DemoVpc