pulumi / upgrade-provider

A tool to automate provider upgrades on your local machine
6 stars 1 forks source link

Use semver sort to determine latest upstream release in getExpectedTargetLatest #220

Closed guineveresaenger closed 8 months ago

guineveresaenger commented 9 months ago

See #218 for details.

Uses the full json repo info payload to determine the latest upstream release.

See https://github.com/pulumi/pulumi-minio/issues/251 for the fix applying to pulumi-minio specifically.

Fixes #218. Fixes https://github.com/pulumi/pulumi-f5bigip/issues/345 Fixes https://github.com/pulumi/pulumi-venafi/issues/244

~Reviewer Note: This functionality overrides the changes in #226. Note that we're returning an empty UpgradeTargetVersion in the case of a non-stable release, so that the nil check on Plan Provider Upgrade reports that we are up to date. A latest version with a beta tag should not fail the upgrade process; we should ignore it.~

VenelinMartinov commented 8 months ago

Thanks for fixing the issue here! I'm not sure this is quite the right logic though. It looks to me like this change yields the following logic: If latest is a beta, do not upgrade.
This is not the correct behaviour in the following example:
1.2-beta latest 1.1 1.0 (current)

We would not upgrade to 1.1 as we should.

Should we instead get the latest version by ordering the list from gh releases?

guineveresaenger commented 8 months ago

I mean, this is all trying to make up for inconsistent human behavior around setting release standards.

Do we think the latest means anything? It sounds like we do not trust upstream to use GitHub latest consistently, as evidenced by a non-stable version being marked as latest on the postgresql provider.

However, I think the case you describe is unlikely:

Likely, what happened here is that a maintainer failed to check "Set as pre-release" when they published their beta version. I would be surprised to see a stable release not setting github latest by default. (It's not impossible).

The reason why I prefer checking things in this way is that there have been several issues in this tool with hand ordering tags, and asking for "what is latest on github" is pretty foolproof from an implementation perspective.

I would argue it is reasonable to assume latest is good, and guard for accidental latest-has-a-prerelease-semver by ignoring it.

edited to add: gh release list does return releases in order...of publish date. This means that the current logic as-is is also potentially broken because it is of course possible to publish a newer-by-date release with an older-by-semver release tag.

guineveresaenger commented 8 months ago

Alright I spoke with @iwahbe about this.

Because (github)latest moves and we only check for updates once a day, we may in fact miss "latest" versions due to latest moving on to a newer tag, and if that tag is not stable semver, we do not upgrade.

However, we also cannot use the first release from gh release list because those are ordered by publish date, not by semver.

We will do a best-attempt semver comparison of the default result of gh release list (30 most recent releases).

...I do not like this. But it is the least lossy option we have. :(

guineveresaenger commented 8 months ago

An upgrade issue with the correct upstream version was opened successfully using this code.

guineveresaenger commented 8 months ago

Hm, the lack of json output is extremely unfortunate. Adjusting to split lines by tab instead to ensure we don't wind up with "Terraform" as our release tag when there is whitespace in release names. This should fix this failure.

gh release list --repo=F5Networks/terraform-provider-bigip --exclude-drafts --exclude-pre-releases
TITLE                              TYPE    TAG NAME  PUBLISHED
Terraform Provider BIG-IP v1.20.1  Latest  v1.20.1   about 1 month ago
Terraform Provider BIG-IP v1.20.0          v1.20.0   about 2 months ago
Terraform Provider BIG-IP v1.19.0          v1.19.0   about 4 months ago
Terraform Provider BIG-IP v1.18.1          v1.18.1   about 5 months ago
Terraform Provider BIG-IP v1.18.0          v1.18.0   about 6 months ago
Terraform Provider BIG-IP v1.17.1          v1.17.1   about 8 months ago
Terraform Provider BIG-IP v1.17.0          v1.17.0   about 9 months ago
Terraform Provider BIG-IP v1.16.2          v1.16.2   about 11 months ago
Terraform Provider BIG-IP v1.16.1          v1.16.1   about 1 year ago
Terraform Provider BIG-IP v1.16.0          v1.16.0   about 1 year ago
Terraform Provider BIG-IP v1.15.2          v1.15.2   about 1 year ago