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

Add jq and curl #23

Merged
merged 1 commit into from
Nov 20, 2023
Merged

Add jq and curl #23

merged 1 commit into from
Nov 20, 2023

Conversation

mogul
Copy link
Contributor

@mogul mogul commented Nov 16, 2023

Changes proposed in this pull request:

  • add jq and curl CLI utils (seems like their absence might be a problem)

Security considerations

None, these are well-understood CLI utilities that are being pulled from the upstream Alpine repository, which is updated frequently.

@markdboyd
Copy link
Contributor

Why does this CLI image need those utilities? How would people use them in an actual CI workflow?

@mogul
Copy link
Contributor Author

mogul commented Nov 16, 2023

The action is also useful for running more complex scripts that assume the caller is already authenticated with cloud.gov and pointing to the appropriate space. Here's an example workflow and the script it points to. In this case the script is mainly using cf curl but you can imagine another one that needs to download a file or reference the result of a request during deployment. For those very simple needs, curl and jq will do.

@FuhuXia
Copy link
Contributor

FuhuXia commented Nov 17, 2023

I would also like to add grep into the list as in

apk add --no-cache bash jq curl grep

We have a workflow step to monitor the output of a cf command, in which a script is relying on grep's --line-buffered option. The BusyBox grep that comes with Alpine version does not support this option. We have to install it in our run script after getting cloud-gov/cg-cli-tools. But installing a GNU grep here, not in our script as done here, is considered better for anyone like our team using grep.

Is it possible to add grep when merging this PR? @mogul @markdboyd

@mogul
Copy link
Contributor Author

mogul commented Nov 17, 2023

I'm okay with that; @markdboyd ...?

Also if there's a common pack of binutil-type stuff not covered by BusyBox, let's just go ahead and install it.

(And: Sorry for the breaking change, @FuhuXia !)

@mogul
Copy link
Contributor Author

mogul commented Nov 17, 2023

@markdboyd
Copy link
Contributor

@mogul sure, feel free to add grep

@markdboyd markdboyd merged commit 51645c7 into cloud-gov:main Nov 20, 2023
1 check passed
@markdboyd markdboyd mentioned this pull request Nov 20, 2023
@markdboyd
Copy link
Contributor

@FuhuXia @mogul I just decided to create a separate PR for including grep: #24

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