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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for regional secrets #300

Conversation

abheda-crest
Copy link
Contributor

@abheda-crest abheda-crest commented Oct 21, 2024

The additional input for region has been added for fetching the regional secret.

It has been tested to verify the functionality, and the secret should be successfully retrieved for both global and regional. Secrets can be referenced using the following formats:

  • projects/<project-id>/secrets/<secret-id>/versions/<version-id>
  • projects/<project-id>/secrets/<secret-id>
  • <project-id>/<secret-id>/<version-id>
  • <project-id>/<secret-id>

Note: Whenever the region input field is provided, the secret will reference to the regional secret, else the global secret will be referred.

More information about regional secrets: https://cloud.google.com/secret-manager/regional-secrets/data-residency

@abheda-crest abheda-crest requested a review from a team as a code owner October 21, 2024 09:51
@sethvargo
Copy link
Member

Hi @abheda-crest - I think I'd prefer to allow users to override the endpoint instead of all this extra string interpolation. You can try this out with the undocumented environment variable GHA_ENDPOINT_OVERRIDE_secretmanager by setting it to something like https://secretmanager.us-east1.rep.googleapis.com/v1, then everything else should just work.

@abheda-crest
Copy link
Contributor Author

Hi @abheda-crest - I think I'd prefer to allow users to override the endpoint instead of all this extra string interpolation. You can try this out with the undocumented environment variable GHA_ENDPOINT_OVERRIDE_secretmanager by setting it to something like https://secretmanager.us-east1.rep.googleapis.com/v1, then everything else should just work.

If we just override endpoint for regional secrets, it won't work out of the box as resource path for regional secret is in following format projects/<project-id>/locations/<location-id>/secrets/<secret-id>/versions/<version-id>. As we have validations placed for resource path, regional secret resource path will fail validation resulting in action/workflow failure.

I thought it would be better to not hamper current validation logic as including location in validation was becoming quite complex. As Current validation allows shorter resource path (i.e <project-id>/<secret-id>/<version-id> or <project-id>/<secret-id>), if we include location in validation of shorter path, extraction/parsing logic from shorter path becomes more erroneous. So I think better option would be to have extra input for region and handle respective endpoint and their resource path based on input rather then just relying on custom endpoint and updated resource path.

let me know if we can address it any other way.

@sethvargo
Copy link
Member

I see. Regional secrets embed the resource location inside the resource name. Do I understand that correctly?

@abheda-crest
Copy link
Contributor Author

I see. Regional secrets embed the resource location inside the resource name. Do I understand that correctly?

Yes, resource path for regional secret includes location/region in it. Following is format for full resource path for regional secret expected at regional secret endpoint projects/<project-id>/locations/<location-id>/secrets/<secret-id>/versions/<version-id>.

If we allow shorter form as we are doing currently including location, we have following possibilities (for both global and regional secrets)
<project-id>/<location-id>/<secret-id>/<version-id>
<project-id>/<location-id>/<secret-id>
<project-id>/<secret-id>/<version-id>
<project-id>/<secret-id>
Here validating and extracting in case of if location is absent or version is absent will be more complex/erroneous.

So due to this I think it would be cleaner approach to have separate region input for handling both regional endpoint and resource path, so that validation on resource path can work more effectively.

Regional API Document for your Reference: https://cloud.google.com/secret-manager/docs/reference/rest/v1/projects.locations.secrets.versions/access

@sethvargo
Copy link
Member

Hey @abheda-crest - I pulled your changes into #301 and made some tweaks. I don't want to accept a new input. Instead we can parse the location from the secret reference and then use that to control the endpoint.

sethvargo added a commit that referenced this pull request Oct 24, 2024
This adds support for regional secrets, but it's intentionally
undocumented. It also adds support for universes, which is documented.

Closes
#300

---------

Co-authored-by: abheda-crest <alfatah.bheda@crestdata.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants