Uh oh!
There was an error while loading. Please reload this page.
[chore] Get rid of all but Azure related secrets. - #2045
Conversation
✅ Deploy Preview will be available once build job completes!
|
TasoOneAsia
left a comment
There was a problem hiding this comment.
Majority of comments are for the new variable names as Azure did not allow for underscores when configuring.
One minor comment around a matrix value evaluating to empty string in some job variations.
| client-id: ${{secrets.DOCS_OIDC_CLIENT}} | ||
| tenant-id: ${{secrets.DOCS_OIDC_TENANT}} | ||
| vault-name: ${{secrets.AZURE_KEY_VAULT_DOCS}} | ||
| secret-names: "slack_webhook_url_docs_incident" |
There was a problem hiding this comment.
| secret-names: "slack_webhook_url_docs_incident" | |
| secret-names: "slackWebhook-docsIncident" |
| client-id: ${{secrets.DOCS_OIDC_CLIENT}} | ||
| tenant-id: ${{secrets.DOCS_OIDC_TENANT}} | ||
| vault-name: ${{secrets.AZURE_KEY_VAULT_DOCS}} | ||
| secret-names: "slack_webhook_url_friends_of_docs" |
There was a problem hiding this comment.
| secret-names: "slack_webhook_url_friends_of_docs" | |
| secret-names: "slackWebhook-friendsOfDocs" |
| - env_name: dev | ||
| env_api_key: "COVEO_API_DEV" | ||
| env_coveo_org_id: "f5networkx1h1607h" | ||
| - env_name: staging | ||
| env_api_key: "COVEO_API_STAGING" | ||
| env_coveo_org_id: "f5networksnonproduction1xqykzabw" | ||
| - env_name: prod | ||
| env_api_key: "COVEO_API_PROD" | ||
| env_sphinx_api_key: "COVEO_SPHINX_API_PROD" | ||
| env_coveo_org_id: "f5networksproduction5vkhn00h" |
There was a problem hiding this comment.
The adjusted names to comply with Azure requirements
- env_name: devenv_api_key: "coveoApi-dev"env_coveo_org_id: "f5networkx1h1607h"
- env_name: stagingenv_api_key: "coveoApi-staging"env_coveo_org_id: "f5networksnonproduction1xqykzabw"
- env_name: prodenv_api_key: "coveoApi-prod"env_sphinx_api_key: "coveoApi-sphinx-prod"env_coveo_org_id: "f5networksproduction5vkhn00h"| - name: Get secrets | ||
| uses: nginx/ci-self-hosted/.github/actions/get-from-vault@refs/heads/main | ||
| with: | ||
| client-id: ${{secrets.DOCS_OIDC_CLIENT}} | ||
| tenant-id: ${{secrets.DOCS_OIDC_TENANT}} | ||
| vault-name: ${{secrets.AZURE_KEY_VAULT_DOCS}} | ||
| secret-names: "${{matrix.env_api_key}}, ${{matrix.env_sphinx_api_key}}, slack_webhook_url_docs_incident" | ||
| env-names: "COVEO_API_KEY, COVEO_SPHINX_API_KEY, SLACK_WEBHOOK_URL" | ||
There was a problem hiding this comment.
${{matrix.env_sphinx_api_key}} evaluates to an empty string in environments other than prod. I'm guessing this is liable to cause issues in the reusable's handling.
If we move that specific call to its own conditional step that gets avoided.
- name: Get secrets (Base)
uses: nginx/ci-self-hosted/.github/actions/get-from-vault@refs/heads/main
with:
client-id: ${{secrets.DOCS_OIDC_CLIENT}}
tenant-id: ${{secrets.DOCS_OIDC_TENANT}}
vault-name: ${{secrets.AZURE_KEY_VAULT_DOCS}}
secret-names: "${{matrix.env_api_key}}, slackWebhook-docsIncident"
env-names: "COVEO_API_KEY, SLACK_WEBHOOK_URL"
- name: Get secrets (SPHINX Prod)
if: ${{ matrix.env_name == "prod" }}
uses: nginx/ci-self-hosted/.github/actions/get-from-vault@refs/heads/main
with:
client-id: ${{secrets.DOCS_OIDC_CLIENT}}
tenant-id: ${{secrets.DOCS_OIDC_TENANT}}
vault-name: ${{secrets.AZURE_KEY_VAULT_DOCS}}
secret-names: "${{matrix.env_sphinx_api_key}}"
env-names: "COVEO_SPHINX_API_KEY"
There was a problem hiding this comment.
What happens if you fetch ${{matrix.env_sphinx_api_key}} in the Base?
There was a problem hiding this comment.
@lamATnginx Haven't explicitly tested but the docs state it'll error out on an env-names argument mismatch or empty secret names.
There was a problem hiding this comment.
Got it. Probably for the better that way from them. Then we yeah we need another step to pull the sphinx api key.
Proposed changes
This PR removes some secrets referenced in workflows in favour of using azure vault.
mend workflow should not be used anymore, so remove it as well.
We should also get rid of either one of OIDC or AZURE set of secrets to avoid further confusion.
Actions in nginxinc/ci-self-hosted may also need update but should be handled separately from this PR
This PR also needs someone to move secrets into the azure vault as I don't know their values.