Remove secret from cicd

[elayrocks]

Description

The motivation of this PR is to remove the client secret in the GitHub actions workflow file cicd.yml to improve security.

To do this, I reference this page and created federated credentials in service principal which allows authentication without the need for explicit client secret. I set up a new repo secret called SECURE_AZURE_CREDENTIALS, which does not contain client secret. Also made corresponding changes to authentication in the workflow.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

I created federated credential for pull-request, so pull-request can use the service principal as well. It shall be deleted after testing.

I tested it by changing pr.yml (link) and make sure az login and az acr login via service principal are successful. But deployment has not been tested. I'm afraid it has to be tested after this PR is merged to main and triggers the CI/CD pipeline.

I created federated credential for this branch remove-secret-from-cicd, so it can use the service principal to authenticate to Azure. It will be deleted after testing. In cicd.yml , changed branch trigger to remove-secret-from-cicd
and comment out the if clause, so that build_and_publish and deploy jobs can all be tested once changes are pushed to remove-secret-from-cicd. Check out the result of latest pipeline run and workflow file

Checklist:

• I have performed a self-review
• Changelog has been updated
• Documentation has been updated
• Unit tests pass locally (./scripts/test)
• Code is linted and styled (./scripts/format)

[mmcfarland]

Looking good, and good idea on the testing technique! I left some comments to dig deeper to deploy script to ensure that it is using the new auth method. You may want to adjust the branch trigger for cicd so that it runs on your PR branch pushes so that you can test out the full execution before merging. There shouldn't be an issue with doing multiple (even failed) deployments against our test infrastructure (is that still right @ghidalgo3 )

[mmcfarland]

LGTM!

I think the required use of use_oidc in the tf may make it difficult for this to be run locally, though that isn't the typical workflow. I think we focus on this secret removal now and shift the burden of local deploys when we cross that bridge.

@elayrocks can you delete the pc-apis secret (make sure to leave the pctasks one for now) before merging this so that the merged run definitely doesn't use the secret? After it's merged, can you delete the old GitHub secrets.AZURE_CREDENTIALS as well?