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.

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

Bump native tf provider to version 5.44.2 #641

Merged
merged 5 commits into from
Nov 4, 2024

Conversation

turkenf
Copy link
Collaborator

@turkenf turkenf commented Oct 28, 2024

Description of your changes

In this PR:

  • Upgraded Terraform provider version to v5.44.2

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Will test with uptest and manually

@turkenf
Copy link
Collaborator Author

turkenf commented Oct 28, 2024

/test-examples="examples/storage/v1beta2/bucket.yaml"

Signed-off-by: Fatih Türken <turkenf@gmail.com>
Signed-off-by: Fatih Türken <turkenf@gmail.com>
Signed-off-by: Cem Mergenci <cmergenci@gmail.com>
Signed-off-by: Cem Mergenci <cmergenci@gmail.com>
@mergenci
Copy link
Collaborator

mergenci commented Nov 1, 2024

/test-examples="examples/storage/v1beta2/bucket.yaml"

@mergenci
Copy link
Collaborator

mergenci commented Nov 1, 2024

/test-examples="examples/storage/v1beta1/bucket.yaml"

@mergenci
Copy link
Collaborator

mergenci commented Nov 1, 2024

/test-examples="examples/container/v1beta1/cluster.yaml"

@mergenci
Copy link
Collaborator

mergenci commented Nov 1, 2024

/test-examples="examples/container/v1beta2/cluster.yaml"

@mergenci
Copy link
Collaborator

mergenci commented Nov 1, 2024

I don't think it was smart to test different versions concurrently 🙂

@mergenci
Copy link
Collaborator

mergenci commented Nov 1, 2024

/test-examples="examples/container/v1beta2/cluster.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 4, 2024

/test-examples="examples/cloudrun/v1beta1/v2job.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 4, 2024

/test-examples="examples/compute/v1beta1/disk.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 4, 2024

/test-examples="examples/compute/v1beta1/healthcheck.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 4, 2024

/test-examples="examples/compute/v1beta1/subnetwork.yaml"

@turkenf
Copy link
Collaborator Author

turkenf commented Nov 4, 2024

/test-examples="examples/container/v1beta1/nodepool.yaml"

@@ -13,6 +13,40 @@ import (
v1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
)

type AdditionalNodeNetworkConfigsInitParameters struct {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should we backport this and other empty structs from the changes in the API of the Cluster.container resource?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We discussed this topic off-channel. Here's the summary: Automatic backporting script begins from "root types", such as ClusterInitParameters, ClusterParameters, and ClusterObservation, and backports only the types that descend from their type hierarchy. Doing so was necessary to handle some edge cases, which caused the “same” struct to have a different name in newer API versions. Because these empty structs, mentioned in the review comment, are not used in anywhere, they are not backported. I argue that we shouldn't backport them manually, simply because doing so won't have any benefits.

@@ -188,7 +188,7 @@ type HealthCheckedTargetsInternalLoadBalancersParameters struct {

// The type of load balancer. This value is case-sensitive. Possible values: ["regionalL4ilb", "regionalL7ilb", "globalL7ilb"]
// +kubebuilder:validation:Optional
LoadBalancerType *string `json:"loadBalancerType" tf:"load_balancer_type,omitempty"`
LoadBalancerType *string `json:"loadBalancerType,omitempty" tf:"load_balancer_type,omitempty"`
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should backport the changes in this file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done. I've seen inconsistencies in generating of omitempty tags. Either there's an edge case that I don't know about or there's a bug that omits generating them. If it's because of a bug and we fix it, we don't need any modifications to our backporting procedure, i.e., we don't need to backport struct tags.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding omitempty tags didn't result in any changes in make generate, which was obvious to me only in hindsight 🙂

Signed-off-by: Cem Mergenci <cmergenci@gmail.com>
@mergenci
Copy link
Collaborator

mergenci commented Nov 4, 2024

/test-examples="examples/container/v1beta1/cluster.yaml"

@turkenf turkenf marked this pull request as ready for review November 4, 2024 12:15
Copy link
Collaborator

@mergenci mergenci left a comment

Choose a reason for hiding this comment

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

LGTM.

@turkenf turkenf merged commit 2cbbcb2 into crossplane-contrib:main Nov 4, 2024
10 checks passed
@turkenf turkenf deleted the bump-tf-5.44.2 branch November 4, 2024 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants