Skip to content

fix: fix fetching rc versions of vault cli #156

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

Merged
merged 4 commits into from
Feb 14, 2024
Merged

fix: fix fetching rc versions of vault cli #156

merged 4 commits into from
Feb 14, 2024

Conversation

matifali
Copy link
Member

@matifali matifali commented Feb 14, 2024

Fixes where the install script was fetching the RC version

@matifali matifali requested a review from mafredri February 14, 2024 12:51
@matifali matifali changed the title fix: fix fetching rc versions on vault fix: fix fetching rc versions of vault cli Feb 14, 2024
Copy link
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

Don't you want to use index.json endpoint?

@matifali
Copy link
Member Author

I was avoiding using jq.

@mtojek
Copy link
Member

mtojek commented Feb 14, 2024

Interesting. Can you shed more light on why? I thought that jq format was more human-friendly.

@matifali
Copy link
Member Author

This is a module, and if a workspace does not have jq, then it will fail. Ideally, we want 0 dependencies if possible, and the module should be useable in any workspace.

@mafredri once suggested having coder util jq|curl|wget|zip|tar type of commands so that we do not depend on anything.

Copy link
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

Gotcha 👍

BTW is grep -P a standard now?

@mafredri
Copy link
Member

This is a module, and if a workspace does not have jq, then it will fail. Ideally, we want 0 dependencies if possible, and the module should be useable in any workspace.

@mafredri once suggested having coder util jq|curl|wget|zip|tar type of commands so that we do not depend on anything.

Yup, this is the way, let's reduce dependencies wherever we can 💪🏻. The coder util is a beautiful idea but we also want to make sure we don't bloat the agent binary. Most of this can be done in a very basic way using the stdlib though, so shouldn't add much weight.

Then again, maybe we don't need to bundle it in coder util, perhaps we can have module dependencies and we have one that's called coder/modules/util that contains a few basic commands, or the ability to download them for the relevant platform.

matifali and others added 2 commits February 14, 2024 16:24
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
@matifali
Copy link
Member Author

matifali commented Feb 14, 2024

@mafredri @mtojek another option is to use
curl -s https://releases.hashicorp.com/vault/index.json | grep -o '"[0-9]\+\.[0-9]\+\.[0-9]\+"' | tr -d '"' | uniq | sort -V | tail -n 1
without using jq it also uses the index.json endpoint

@mtojek
Copy link
Member

mtojek commented Feb 14, 2024

another option is to use

I think I like the first option, curl + regular endpoint + grep. As index.json is one-line response, I wouldn't parse it without jq.

@matifali matifali merged commit d10ce91 into main Feb 14, 2024
@matifali matifali deleted the vault-version branch February 14, 2024 14:04
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 this pull request may close these issues.

3 participants