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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci-credentials: update GSM secrets with updated configuration values #20076

Merged

Conversation

alafanechere
Copy link
Contributor

@alafanechere alafanechere commented Dec 5, 2022

What

Closes #19441 (2/2)

#19979 enabled SAT to write updated configurations by capturing CONTROL messages.
When running in the CI, we need to upload these updated configurations to GSM. This is the scope of this PR.

How

  1. Refactor ci-credentials to expose two commands via click framework:
  • write-to-storage: the already existing command that downloads GSM secrets locally for test runs
  • update-secrets: the new command to update GSM secrets according to the content of the secrets/updated_configurations folder that might contain SAT updated configs. This command finds the latest local updated configuration and updates the existing GSM secret by creating a new secret version and disabling the previous one.
    def ci_credentials(ctx, connector_name: str, gcp_gsm_credentials):
  1. Create Secret and RemoteSecret models for convenient manipulation of secret structure.

  2. Rename SecretLoader to SecretManager and add methods to update secrets and disable version.

def update_secrets(self, existing_secrets: List[RemoteSecret]) -> int:

  1. Test SecretManager public methods and models:

def test_read(matchers, connector_name, gsm_secrets, expected_secrets):

def test_secret_instantiation(connector_name, filename, expected_name, expected_directory):

  1. Change Github workflows using ci-credentials to make them call the new update-secrets commands after test run.

馃毃 User Impact 馃毃

This PR changes the /test and /publish workflows, so we shall make sure this command are still functioning as expected.

@alafanechere alafanechere temporarily deployed to more-secrets December 5, 2022 16:50 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets December 6, 2022 09:54 Inactive
@alafanechere alafanechere temporarily deployed to more-secrets December 6, 2022 10:05 Inactive
@alafanechere alafanechere marked this pull request as ready for review December 6, 2022 10:15
@alafanechere alafanechere temporarily deployed to more-secrets December 6, 2022 10:55 Inactive
@alafanechere
Copy link
Contributor Author

alafanechere commented Dec 6, 2022

I manually dispatched a test here on source-facebook-marketing to check that the updated test-command workflow is still working. I could not use /test command dispatch as it checkouts the workflow from master.

Update: the test-command workflow ran successfully 馃帀 so the test run on existing connector (not emitting CONTROL message) are likely to not be disturbed by my changes.

@evantahler
Copy link
Contributor

https://click.palletsprojects.com/en/8.1.x/ is neat!

Copy link
Contributor

@evantahler evantahler left a comment

Choose a reason for hiding this comment

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

Awesome work! Just one main question below, while waiting on others who know more about 馃悕


@click.group()
@click.argument("connector_name")
@click.option("--gcp-gsm-credentials", envvar="GCP_GSM_CREDENTIALS")
Copy link
Contributor

Choose a reason for hiding this comment

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

Confirming that GCP_GSM_CREDENTIALS has read and write access?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not allowed to check in GitHub which service account is used in this env var in our actions. But according to the service account I see in our IAM console, I'm pretty sure the currently used SA has sufficient permissions to perform all the operations that this CLI can do: read secret values, add a secret version, disable a secret version.

@evantahler evantahler requested a review from a team December 6, 2022 21:14
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

Props for refactoring and adding tests. This is great for maintainability

tools/ci_credentials/ci_credentials/secrets_manager.py Outdated Show resolved Hide resolved
tools/ci_credentials/ci_credentials/models.py Outdated Show resolved Hide resolved
@alafanechere alafanechere temporarily deployed to more-secrets December 7, 2022 09:37 — with GitHub Actions Inactive
@alafanechere alafanechere temporarily deployed to more-secrets December 7, 2022 09:42 — with GitHub Actions Inactive
@alafanechere alafanechere temporarily deployed to more-secrets December 7, 2022 09:45 — with GitHub Actions Inactive
@alafanechere
Copy link
Contributor Author

https://click.palletsprojects.com/en/8.1.x/ is neat!

Yup, it currently powers octavia-cli and cloud-cli.

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.

Integration Tests for Connectors that have short-lived credentials can update thier configs
3 participants