arkade icon indicating copy to clipboard operation
arkade copied to clipboard

Use text/template instead of html/template in url building

Open mikhail-sakhnov opened this issue 3 years ago • 4 comments

Now the "html/template" is used for templating URLs in get subcommand. That leads to unnecessary escaping of the URL parts.

For demonstrating the expected and current behaviours I use the following patch applied which is not a part of this PR This patch adds k0s project as a tool to download by arkade.

diff --git a/pkg/get/tools.go b/pkg/get/tools.go
index 5feedb9..e394d63 100644
--- a/pkg/get/tools.go
+++ b/pkg/get/tools.go
@@ -1283,5 +1283,24 @@ https://github.com/{{.Owner}}/{{.Repo}}/releases/download/{{.Version}}/{{.Name}}
 			`,
 		},
 	)
+
+	tools = append(tools,
+		Tool{
+			Owner:       "k0sproject",
+			Repo:        "k0s",
+			Name:        "k0s",
+			Description: "Zero Friction Kubernetes",
+			BinaryTemplate: `
+			{{$arch := ""}}
+			{{- if eq .Arch "x86_64" -}}
+			{{$arch = "amd64"}}
+			{{- else if eq .Arch "aarch64" -}}
+			{{$arch = "arm64"}}
+			{{- end -}}
+			{{.Name}}-{{.Version}}-{{$arch}}
+			`,
+		},
+	)
+
 	return tools
 }

Expected Behaviour

The version scheme of the k0s project is built like:

  • +<kubernetes_version> I expect the final url be like this
➜  arkade git:(master) ✗ go run main.go get k0s
Downloading k0s
https://github.com/k0sproject/k0s/releases/download/v1.21.2+k0s.1/k0s-v1.21.2+k0s.1-amd64

Current Behaviour

However because of the html/template the url is built like this

➜  arkade git:(9575ed0) ✗ go run main.go get k0s
Downloading k0s
https://github.com/k0sproject/k0s/releases/download/v1.21.2+k0s.1/k0s-v1.21.2&#43;k0s.1-amd64

Possible Solution

Change templating engine from html/template to text/template

Steps to Reproduce (for bugs)

  1. add a new tool with special sign ("+" for example) as a part of the templated string
  2. try to download that tool

Context

I was trying to add support of the https://k0sproject.io tool The support of the k0s tool is not a part of that PR.

Your Environment

  • What Kubernetes distribution are you using?

applicable

  • Operating System and version (e.g. Linux, Windows, MacOS):
Darwin mbp13 20.5.0 Darwin Kernel Version 20.5.0: Sat May  8 05:10:33 PDT 2021; root:xnu-7195.121.3~9/RELEASE_X86_64 x86_64 i386 MacBookPro16,2 Darwin
  • What arkade version is this?

main head

Are you a GitHub Sponsor (Yes/No?)

Check at: https://github.com/sponsors/alexellis

  • [ ] Yes
  • [X] No

Should be fixed in #447

mikhail-sakhnov avatar Jul 07 '21 08:07 mikhail-sakhnov

Hi there,

That's a perfect example, and exactly what we're looking for in contributions. Issue first, discussion, then PR if requested.

Can you as part of your PR add the K0s project, and the requisite unit test? Just rename your PR to "adding K0s project" and follow the example given elsewhere for unit-testing.

In that way, your patch is warranted, but if we have no examples that fail, then it is useful, but of theoretical benefit.

Alex

alexellis avatar Jul 07 '21 08:07 alexellis

@alexellis sure, I will modify the PR accordingly. Do you have any way to prevent some tool from being listed under specific problem? The k0s binary doesn't work on darwin

mikhail-sakhnov avatar Jul 07 '21 11:07 mikhail-sakhnov

@alexellis I added two new binaries as a part of the PR (k0s and k0sctl) + rebased with the actual main branch

mikhail-sakhnov avatar Jul 12 '21 11:07 mikhail-sakhnov

Thank you, please see the latest comments on your PR https://github.com/alexellis/arkade/pull/447

alexellis avatar Jul 15 '21 08:07 alexellis

Since PR #447 was merged for almost a year ago and it is working great.

I'll close this if you don't mind.

Shikachuu avatar Aug 12 '22 15:08 Shikachuu

/close

Shikachuu avatar Aug 12 '22 15:08 Shikachuu