Tweaking URL regex based on comment from aderuelle #225

micahflattbuilt · 2022-05-09T21:23:31Z

URL tweaks based on changes to the hashicorp release site.

allen-servedio · 2022-05-09T21:28:02Z

@micahflattbuilt - sorry, too fast on that last comment. Confirmed that it works for:

terraform {
  required_version = ">= 1.0, < 2.0"
}

Reading required version from terraform file
Reading required version from constraint: >= 1.0, < 2.0
Matched version: 1.1.9
Creating directory for terraform binary at: /Users/user/.terraform.versions
Downloading to: /Users/user/.terraform.versions
20709155 bytes downloaded
Switched terraform to version "1.1.9"

allen-servedio · 2022-05-09T21:39:22Z

No @micahflattbuilt - I goofed and did not copy https://github.com/warrensbox/terraform-switcher/pull/225/files#diff-99b146ff8d948c450dfa0fe84cb71461f9b43acbfc52837fbf712e696ccea2f2R40 (a little too fast off the trigger). I am trying to get our automation back up...

micahflattbuilt · 2022-05-09T21:54:45Z

I know all about trying to get your automation back up quickly... I posted my build to our CD servers to get things rolling again (with success). It's a bit of a silly patch but so far seems to work as a quick fix.

sw-popop · 2022-05-09T22:12:30Z

^^^ merge please

franviera92 · 2022-05-09T22:34:03Z

we need this change please! merge :(

jukie · 2022-05-09T22:54:58Z

I pushed a temp release to my fork with these changes in the meantime - https://github.com/Jukie/terraform-switcher/releases/tag/0.13.12345

yermulnik · 2022-05-09T23:33:12Z

lib/list_versions.go

@@ -27,16 +27,17 @@ func GetTFList(mirrorURL string, preRelease bool) ([]string, error) {
 	var semver string
 	if preRelease == true {
 		// Getting versions from body; should return match /X.X.X-@/ where X is a number,@ is a word character between a-z or A-Z
-		semver = `\/(\d+\.\d+\.\d+)(-[a-zA-z]+\d*)?\/`
+		semver = `\/(\d+\.\d+\.\d+)(-[a-zA-z]+\d*)?\"`


out of interest: what is the reason to quote double quote in regex? does it have any special meaning? or is this used down the code and is enclosed in double quotes? (the latter explain the need to quote)
also might probably need to adjust comment above this line to replace last char of regex.

escaping is not needed in raw string literals

could use word characters class \w for the '@' part

we also could accomodate for possible reintroduction of trailing '/'

Not tested:

Suggested change

semver = `\/(\d+\.\d+\.\d+)(-[a-zA-z]+\d*)?\"`

semver = `/(\d+.\d+.\d+)(-\w+)?/?"`

Without the double quote in the regex, I was finding that it was capturing both pre release and full release versions every time. The double quote acts as an ending delimiter so that the right versions are found when ignoring pre-release versions.

~~Though the double quote doesn't need to be escaped, right?~~ Oops, please disregard, I didn't make it to see the new commit to this PR by the time of writing the comment.

yermulnik · 2022-05-09T23:34:21Z

lib/list_versions.go

 	}
 	r, _ := regexp.Compile(semver)
 	for i := range result {
 		if r.MatchString(result[i]) {
 			str := r.FindString(result[i])
-			trimstr := strings.Trim(str, "/") //remove "/" from /X.X.X/
+			trimstr := strings.Trim(str, "/\"") //remove "/" from /X.X.X/


Might probably need to adjust comment on this line to reflect regex pattern change.

yermulnik · 2022-05-09T23:46:18Z

Also may probably worth to switch to their new API?
https://www.hashicorp.com/blog/announcing-the-hashicorp-releases-api

alxdrl · 2022-05-10T05:15:36Z

lib/list_versions.go

 	} else if preRelease == false {
 		// Getting versions from body; should return match /X.X.X/ where X is a number
-		semver = `\/(\d+\.\d+\.\d+)\/`
+		// without the ending '"' pre-release folders would be tried and break.
+		semver = `\/(\d+\.\d+\.\d+)\"`


Ditto, not tested:

Suggested change

semver = `\/(\d+\.\d+\.\d+)\"`

semver = `/(\d+.\d+.\d+)/?"`

Now works on interactive mode. Thank you.

Removed the escape of the double quote and added the optional ending slash. My local tests still work with the additions.

roys8 · 2022-05-10T06:59:25Z

Hello Team,

May we expect this to be available today, we have some CRQ going and we got stuck due to this 😄

smailc · 2022-05-10T08:55:56Z

+1

JanisOrlovs · 2022-05-10T09:26:48Z

+1

toniokaluza · 2022-05-10T10:31:38Z

+1

andrewinci · 2022-05-10T10:57:17Z

+1

kstormp · 2022-05-10T12:08:06Z

+1

franviera92 · 2022-05-10T12:34:24Z

No one assigned (15h)

…nding slash

micahflattbuilt · 2022-05-10T13:16:52Z

@yermulnik , I agree that the release API would be the way to go. This PR just tries to provide a quick path for recovery.

kizzie · 2022-05-10T13:20:01Z

No one assigned (15h)

I expect there are timezone differences to see as well :) but bumping this some more!

yermulnik · 2022-05-10T14:24:55Z

@yermulnik , I agree that the release API would be the way to go. This PR just tries to provide a quick path for recovery.

Got you. Makes sense. Thanks.

smailc · 2022-05-10T15:15:04Z

Is it going to be merged today? I need to know if we need to change our build tools to not use tfswitch to make them work

mantoniocc · 2022-05-10T15:21:49Z

I pushed a temp release to my fork with these changes in the meantime - https://github.com/Jukie/terraform-switcher/releases/tag/0.13.12345

We are using temporally @jukie fork in our pipelines and work fine!

We apply the following fix to our docker images.
curl -O -L https://github.com/Jukie/terraform-switcher/releases/download/0.13.12345/terraform-switcher_0.13.12345_linux_amd64.tar.gz && \ tar -xvzf terraform-switcher_0.13.12345_linux_amd64.tar.gz && \ mv tfswitch /bin/tfswitch

…iner There is a current issue with the original tfswitch as hashicorp changed their website and so the bit of tfswitch which scrapes that info is failing. This is a temporary fix until the PR is merged on tfswitch. github issue: warrensbox/terraform-switcher#225 differences between versions: warrensbox/terraform-switcher@master...Jukie:fix-release-downloads

gabelazo · 2022-05-10T16:00:59Z

We switched to tfenv. https://github.com/tfutils/tfenv

llamahunter · 2022-05-10T19:13:29Z

@warrensbox any chance of getting this merged soon?

allen-servedio · 2022-05-10T19:30:15Z

@micahflattbuilt - I found a couple more places around the "pre-release" tests:

From 64b541bb42d23ae395cfd5b28f77e9de6724b2c9 Mon Sep 17 00:00:00 2001
From: Allen Servedio
Date: Tue, 10 May 2022 15:19:29 -0400
Subject: [PATCH] Fix pre-versions

---
 lib/list_versions.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/list_versions.go b/lib/list_versions.go
index c072955..1802e5e 100644
--- a/lib/list_versions.go
+++ b/lib/list_versions.go
@@ -58,12 +58,12 @@ func GetTFLatest(mirrorURL string) (string, error) {
 		return "", error
 	}
 	// Getting versions from body; should return match /X.X.X/ where X is a number
-	semver := `\/(\d+\.\d+\.\d+)\/`
+	semver := `\/(\d+\.\d+\.\d+)\/?"`
 	r, _ := regexp.Compile(semver)
 	for i := range result {
 		if r.MatchString(result[i]) {
 			str := r.FindString(result[i])
-			trimstr := strings.Trim(str, "/") //remove "/" from /X.X.X/
+			trimstr := strings.Trim(str, "/\"") //remove '/' or '"' from /X.X.X/" or /X.X.X"
 			return trimstr, nil
 		}
 	}
@@ -81,7 +81,7 @@ func GetTFLatestImplicit(mirrorURL string, preRelease bool, version string) (str
 			return "", error
 		}
 		// Getting versions from body; should return match /X.X.X-@/ where X is a number,@ is a word character between a-z or A-Z
-		semver := fmt.Sprintf(`\/(%s{1}\.\d+\-[a-zA-z]+\d*)\/`, version)
+		semver := fmt.Sprintf(`\/(%s{1}\.\d+\-[a-zA-z]+\d*)\/?"`, version)
 		r, err := regexp.Compile(semver)
 		if err != nil {
 			return "", err
@@ -89,7 +89,7 @@ func GetTFLatestImplicit(mirrorURL string, preRelease bool, version string) (str
 		for i := range versions {
 			if r.MatchString(versions[i]) {
 				str := r.FindString(versions[i])
-				trimstr := strings.Trim(str, "/") //remove "/" from /X.X.X/
+				trimstr := strings.Trim(str, "/\"") //remove '/' or '"' from /X.X.X/" or /X.X.X"
 				return trimstr, nil
 			}
 		}
--
2.32.0 (Apple Git-132)

micahflattbuilt · 2022-05-10T19:55:05Z

I found a couple more places around the "pre-release" tests:

@allen-servedio applied your diff as a new commit.

jukie · 2022-05-11T04:29:38Z

Sorry to piggyback the PR but was anyone able to get the releases api working for anything past the most recent releases(e.g. first batch from whatever's passed in for limit)?
after doesn't appear to be working as intended.

jphuynh · 2022-05-11T09:48:53Z

@warrensbox Is it possible to release this as a temporary hotfix?

I appreciate that there are conversations around a more reliable implementation using the releases API but suppose that can be addressed in a further release. A hotfix from this PR would be great in the mean time.

alxdrl

LGTM

leolorenzoluis

LGTM! MERGE😜! 🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀

github issue: warrensbox/terraform-switcher#225 differences between versions: warrensbox/[email protected]:fix-release-downloads

smailc · 2022-05-11T13:19:50Z

Thanks @jukie - fix works great

warrensbox · 2022-05-11T21:48:20Z

https://www.hashicorp.com/blog/announcing-the-hashicorp-releases-api

This is fixed and released

Tweaking URL regex based on comment from aderuelle

a9507db

micahflattbuilt mentioned this pull request May 9, 2022

Stopped working inside terraform project #224

Closed

allen-servedio pushed a commit to TheWeatherCompany/terraform-switcher that referenced this pull request May 9, 2022

Applying fixes from warrensbox#225

5975412

yermulnik reviewed May 9, 2022

View reviewed changes

alxdrl reviewed May 10, 2022

View reviewed changes

Simplify the regex and account for possiblity of re-introduction of e…

c0b7bc6

…nding slash

This was referenced May 10, 2022

Update to using the Jukie repo for tfswitch in terraform docker container ovotech/circleci-orbs#427

Closed

Update to using the Jukie repo for tfswitch in terraform docker container ovotech/circleci-orbs#428

Merged

allen-servedio pushed a commit to TheWeatherCompany/terraform-switcher that referenced this pull request May 10, 2022

Updating with latest changes from warrensbox#225

ec0cf55

Applying patch from allen-servedio

7282aec

jukie approved these changes May 11, 2022

View reviewed changes

cvalentin-dkt approved these changes May 11, 2022

View reviewed changes

toniokaluza approved these changes May 11, 2022

View reviewed changes

leolorenzoluis approved these changes May 11, 2022

View reviewed changes

alxdrl approved these changes May 11, 2022

View reviewed changes

leolorenzoluis approved these changes May 11, 2022

View reviewed changes

kizzie mentioned this pull request May 11, 2022

Use Jukie repo for tfswitch temporarily for the terraform-v1 orbs ovotech/circleci-orbs#430

Merged

gartoty approved these changes May 11, 2022

View reviewed changes

warrensbox merged commit 31a5b91 into warrensbox:master May 11, 2022

	semver = `\/(\d+\.\d+\.\d+)(-[a-zA-z]+\d*)?\"`
	semver = `/(\d+.\d+.\d+)(-\w+)?/?"`

Tweaking URL regex based on comment from aderuelle #225

Tweaking URL regex based on comment from aderuelle #225

Conversation

micahflattbuilt commented May 9, 2022

allen-servedio commented May 9, 2022 • edited

allen-servedio commented May 9, 2022

micahflattbuilt commented May 9, 2022

sw-popop commented May 9, 2022

franviera92 commented May 9, 2022

jukie commented May 9, 2022

yermulnik May 9, 2022

Choose a reason for hiding this comment

alxdrl May 10, 2022

Choose a reason for hiding this comment

micahflattbuilt May 10, 2022

Choose a reason for hiding this comment

yermulnik May 10, 2022 • edited

Choose a reason for hiding this comment

yermulnik May 9, 2022

Choose a reason for hiding this comment

yermulnik commented May 9, 2022

alxdrl May 10, 2022

Choose a reason for hiding this comment

rixiet May 10, 2022

Choose a reason for hiding this comment

micahflattbuilt May 10, 2022

Choose a reason for hiding this comment

roys8 commented May 10, 2022

smailc commented May 10, 2022

JanisOrlovs commented May 10, 2022

toniokaluza commented May 10, 2022

andrewinci commented May 10, 2022

kstormp commented May 10, 2022

franviera92 commented May 10, 2022

micahflattbuilt commented May 10, 2022

kizzie commented May 10, 2022

yermulnik commented May 10, 2022 • edited

smailc commented May 10, 2022

mantoniocc commented May 10, 2022 • edited

gabelazo commented May 10, 2022

llamahunter commented May 10, 2022

allen-servedio commented May 10, 2022

micahflattbuilt commented May 10, 2022

jukie commented May 11, 2022 • edited

jphuynh commented May 11, 2022

alxdrl left a comment

Choose a reason for hiding this comment

leolorenzoluis left a comment

Choose a reason for hiding this comment

smailc commented May 11, 2022

warrensbox commented May 11, 2022

allen-servedio commented May 9, 2022 •

edited

yermulnik May 10, 2022 •

edited

yermulnik commented May 10, 2022 •

edited

mantoniocc commented May 10, 2022 •

edited

jukie commented May 11, 2022 •

edited