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

Upgrade terraform-provider-equinix to v1.34.0 #89

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

ocobles
Copy link
Contributor

@ocobles ocobles commented Apr 2, 2024

This PR was generated via $ upgrade-provider equinix/pulumi-equinix --upstream-provider-name terraform-provider-equinix.


  • Upgrading terraform-provider-equinix from 1.32.0 to 1.34.0.
  • Upgrading pulumi-terraform-bridge from v3.77.0 to v3.79.0.
  • Upgrading pulumi-terraform-bridge/pf from v0.30.0 to v0.32.0.

@ocobles ocobles self-assigned this Apr 2, 2024
@ocobles ocobles requested a review from a team April 2, 2024 13:55
Copy link

github-actions bot commented Apr 2, 2024

Does the PR have any schema changes?

Does the PR have any schema changes?

Found 4 breaking changes:

Resources

  • 🟢 "equinix:metal/organization:Organization": required: "address" property is no longer Required

Types

  • 🟡 "equinix:fabric/ConnectionASideAccessPointPort:ConnectionASideAccessPointPort": properties: "redundancies" missing
  • 🟡 "equinix:fabric/ConnectionZSideAccessPointPort:ConnectionZSideAccessPointPort": properties: "redundancies" missing
  • 🟢 "equinix:metal/getProjectBgpConfig:getProjectBgpConfig": required: "md5" property has changed to Required
    No new resources/functions.

Maintainer note: consult the runbook for dealing with any breaking changes.

@@ -31,7 +29,7 @@ public final class GetProjectBgpConfig {
* @return Password for BGP session in plaintext (not a checksum).
*
*/
private @Nullable String md5;
private String md5;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what this change corresponds to on the terraform side. The md5 attribute is still optional in the terraform provider, so I would expect it to remain nullable in Pulumi.

Copy link
Contributor Author

@ocobles ocobles Apr 2, 2024

Choose a reason for hiding this comment

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

That's the output, and we are setting always a value for BGPConfig in https://github.com/equinix/terraform-provider-equinix/blob/d63009a723d5f9101cf7ee7ede863eef9a4fb324/internal/resources/metal/project/models.go#L97

even if it is an empty list so it makes sense for me.

In the project resource definition is still Nullable

private Output</* @Nullable */ ProjectBgpConfig> bgpConfig;

Copy link
Contributor

Choose a reason for hiding this comment

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

When using the terraform provider directly, at least, MD5 should still be nil on output if the user provides a BGP config but does not specify an MD5; that behavior was restored in 1.33.1 and should continue to exist in 1.34.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm on board to write this off as a Pulumi thing, but my concern is that this may indicate there's still something wrong with how we're handling null MD5 in the Terraform provider.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you right I was checking bgpConfig instead of the specific md5 field

Copy link
Contributor

Choose a reason for hiding this comment

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

It sounds like this is a reflection of the issue we ran into during the TF framework provider conversion, where we couldn't define a nested schema for BGP config in the data source, so it doesn't have the same behavior as the converted resource. If that's the case, this is expected and there's not really anything we can do about it.

@ctreatma ctreatma merged commit c115ad8 into main Apr 2, 2024
15 checks passed
@ctreatma ctreatma deleted the upgrade-terraform-provider-equinix-to-v1.34.0 branch August 21, 2024 14:52
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