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

findRelease() assumes usage is only for kubeseal #935

Closed
1 of 2 tasks
rgee0 opened this issue Sep 25, 2022 · 3 comments
Closed
1 of 2 tasks

findRelease() assumes usage is only for kubeseal #935

rgee0 opened this issue Sep 25, 2022 · 3 comments

Comments

@rgee0
Copy link
Contributor

rgee0 commented Sep 25, 2022

This is loosely related to #874 and is extant in the current CLI version.

Expected Behaviour

If a faas-cli version check fails the error message that bubbles up to the user should be specific to faas-cli

Current Behaviour

Due to https:/openfaas/faas-cli/blob/master/commands/cloud.go#L194 if a faas-cli version check fails due to a zero length location header the error message returned to the user will refer to kubeseal.

Why do you need this?

Clarity for the user

Are you a GitHub Sponsor (Yes/No?)

Check at: https:/sponsors/openfaas

  • Yes
  • No

List All Possible Solutions and Workarounds

Swap out the string literal that refers to the tool name and use the incoming url value instead

Which Solution Do You Recommend?

Swap out the string literal that refers to the tool name and use the incoming url value instead

Steps to Reproduce (for bugs)

Difficult as it requires the specific error condition to be forced. Its easier understood through code inspection.

This call:

latest, err := findRelease(releases)

Reaches:
func findRelease(url string) (string, error) {

which returns:
return "", fmt.Errorf("unable to determine release of kubeseal")

Context

This is something I spotted while working on #874 and is more of a consideration if #874 doesn't get merged. In any event id suggest findReleases() should be made more generic.

Your Environment

  • FaaS-CLI version ( Full output from: faas-cli version ):

  • Docker version ( Full output from: docker version ):

  • Are you using Docker Swarm (FaaS-swarm ) or Kubernetes (FaaS-netes)?

  • Operating System and version (e.g. Linux, Windows, MacOS):

  • Link to your project or a code example to reproduce issue:

@alexellis
Copy link
Member

Is there a package in arkade that we can vendor for this?

@rgee0
Copy link
Contributor Author

rgee0 commented Oct 3, 2022

@alexellis
Copy link
Member

I've made the suggested change, thank you for looking into it

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 a pull request may close this issue.

2 participants