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

terraform-provider: require kubernetes and microservice version #2791

Merged
merged 3 commits into from
Jan 4, 2024

Conversation

elchead
Copy link
Contributor

@elchead elchead commented Jan 3, 2024

Context

The provider is currently using a default when no version is specified for these fields. This however leads to an unexpected upgrade state, because new versions defaults, as part of provider upgrades, are not seen as state change by Terraform. Consequently, the new defaults are not applied. This is especially undesired for the microservice version, which should always coincide with the provider version.

Proposed change(s)

  • marking as breaking change to make users aware
  • make fields required
  • adjust provider e2e test

Additional info

Checklist

  • Update docs
  • Add labels (e.g., for changelog category)
  • Is PR title adequate for changelog?
  • Link to Milestone

Copy link

netlify bot commented Jan 3, 2024

Deploy Preview for constellation-docs canceled.

Name Link
🔨 Latest commit b5b684b
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/659681bcc7fe570008f1fe36

@elchead elchead added the breaking change Change breaks existing API or configuration. label Jan 3, 2024
@elchead elchead requested a review from msanft January 3, 2024 14:36
@elchead elchead added this to the v2.15.0 milestone Jan 3, 2024
@elchead elchead marked this pull request as ready for review January 3, 2024 14:36
@elchead elchead force-pushed the fix/terraform-provider/require-versions branch from 875f97d to fc262e2 Compare January 3, 2024 14:37
@elchead elchead force-pushed the fix/terraform-provider/require-versions branch from fc262e2 to 8fcdccb Compare January 3, 2024 14:53
@elchead elchead force-pushed the fix/terraform-provider/require-versions branch 3 times, most recently from 494472c to 958781c Compare January 4, 2024 09:42
@elchead elchead requested a review from msanft January 4, 2024 09:42
@elchead
Copy link
Contributor Author

elchead commented Jan 4, 2024

Running E2E

@elchead elchead force-pushed the fix/terraform-provider/require-versions branch from 958781c to b5b684b Compare January 4, 2024 10:00
Copy link
Contributor

github-actions bot commented Jan 4, 2024

Coverage report

Package Old New Trend
terraform-provider-constellation/internal/provider 7.40% 7.00% ↘️

msanft
msanft previously requested changes Jan 4, 2024
Copy link
Contributor

@msanft msanft left a comment

Choose a reason for hiding this comment

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

LGTM except for the one comment

name = "constell"
version = "vX.Y.Z"
kubernetes_version = "vX.Y.Z"
microservice_version = "vX.Y.Z"
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be equal to version and thus obsolete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no. version refers to the image_version is and is renamed to make this more clear in #2775

@elchead elchead merged commit f41ce43 into main Jan 4, 2024
11 checks passed
@elchead elchead deleted the fix/terraform-provider/require-versions branch January 4, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Change breaks existing API or configuration.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants