-
Notifications
You must be signed in to change notification settings - Fork 83
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
docs: allow kubectl apply -f
through a URL
#253
Conversation
docs/book/src/installation.md
Outdated
##### `go install` | ||
|
||
```bash | ||
sed -i "s/AZURE_TENANT_ID: .*/AZURE_TENANT_ID: ${AZURE_TENANT_ID}/" deploy/azure-wi-webhook.yaml | ||
sed -i "s/AZURE_ENVIRONMENT: .*/AZURE_ENVIRONMENT: ${AZURE_ENVIRONMENT}/" deploy/azure-wi-webhook.yaml | ||
kubectl apply -f deploy/azure-wi-webhook.yaml | ||
go install github.com/a8m/envsubst/cmd/envsubst@v1.2.0 | ||
``` | ||
|
||
##### `curl` | ||
|
||
###### Linux and MacOS | ||
|
||
```bash | ||
curl -L https://github.com/a8m/envsubst/releases/download/v1.2.0/envsubst-$(uname -s)-$(uname -m) -o envsubst | ||
chmod +x envsubst | ||
sudo mv envsubst /usr/local/bin | ||
``` | ||
|
||
###### Windows | ||
|
||
```bash | ||
curl -L https://github.com/a8m/envsubst/releases/download/v1.2.0/envsubst.exe | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we need to have the detailed steps for installing envsubst across different environments. We would need to keep this up-to-date anytime there is a change in the release. The installation instruction in the envsubst
repo already has these details: https://github.com/a8m/envsubst#installation, so should we just add a link to that instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good
docs/book/src/installation.md
Outdated
helm install workload-identity-webhook manifest_staging/charts/workload-identity-webhook \ | ||
--namespace azure-workload-identity-system \ | ||
--create-namespace \ | ||
--set azureTenantID="${AZURE_TENANT_ID}" | ||
--set azureTenantID="${AZURE_TENANT_ID}" \ | ||
--set azureEnvironment="${AZURE_ENVIRONMENT}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to add defaulting here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already default to AzurePublicCloud
in values.yaml
but I want to be explicit and clear in the installation step in case we support different clouds in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a specialized use case. Should we simplify it by removing it then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I can add it back once custom cloud is supported
Signed-off-by: Ernest Wong <chuwon@microsoft.com>
Reason for Change:
Allow
kubectl apply -f
through a URL.Requirements
Issue Fixed:
Fixes #250
Please answer the following questions with yes/no:
Does this change contain code from or inspired by another project? If so, did you notify the maintainers and provide attribution?
Notes for Reviewers: