Skip to content
This repository has been archived by the owner on Jan 25, 2023. It is now read-only.

Added the ability to pass arbitrary headers. #187

Closed
wants to merge 6 commits into from
Closed

Added the ability to pass arbitrary headers. #187

wants to merge 6 commits into from

Conversation

omadawn
Copy link

@omadawn omadawn commented Aug 13, 2020

Added a --header | -H option to allow the passing of arbitrary headers.

Fixes #186

@omadawn omadawn requested a review from robmorgan as a code owner August 13, 2020 00:40
@hashicorp-cla
Copy link

hashicorp-cla commented Aug 13, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

echo -e " --ca-file-path\t\tPath to a PEM-encoded certificate authority used to encrypt and verify authenticity of client and server connections. Will be installed under <install-path>/tls/ca."
echo -e " --cert-file-path\t\tPath to a PEM-encoded certificate, which will be provided to clients or servers to verify the agent's authenticity. Will be installed under <install-path>/tls. Must be provided along with --key-file-path."
echo -e " --key-file-path\t\tPath to a PEM-encoded private key, used with the certificate to verify the agent's authenticity. Will be installed under <install-path>/tls. Must be provided along with --cert-file-path"
echo -e " --help | -h\t\tPrints this usage message."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would you mind moving the single-param options (e.g., -h and -v) to a separate PR? It makes it easier to review, release, revert, debug, etc. Thanks!

Copy link
Author

Choose a reason for hiding this comment

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

I'll try and get to that this weekend.

modules/install-consul/install-consul Outdated Show resolved Hide resolved
modules/install-consul/install-consul Outdated Show resolved Hide resolved
modules/install-consul/README.md Outdated Show resolved Hide resolved
modules/install-consul/README.md Show resolved Hide resolved
modules/install-consul/install-consul Outdated Show resolved Hide resolved

if [[ -z "$download_url" && -n "$version" ]]; then
download_url="https://releases.hashicorp.com/consul/${version}/consul_${version}_linux_amd64.zip"
fi

if [[ ! -z "$addtl_headers" ]]; then
local headers
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this declaration outside the if-statement.

modules/install-consul/install-consul Outdated Show resolved Hide resolved

for value in "${addtl_headers[@]}"
do
headers+="-H \"$value\" "
Copy link
Collaborator

Choose a reason for hiding this comment

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

Slightly cleaner to make headers an array (declare it up above local -a headers=()), appending to it here via headers+=("-H \"$value\""), and expanding it in the curl command below with ${headers[@]}.

omadawn and others added 5 commits August 14, 2020 11:27
According to google arrays are always local so that bit is redundant but I like it for consistency.

Co-authored-by: Yevgeniy Brikman <[email protected]>
Meant to add that, ty.

Co-authored-by: Yevgeniy Brikman <[email protected]>
@omadawn omadawn closed this by deleting the head repository Aug 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow passing arbitrary headers
3 participants