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

Improve secret mounting instructions #318

Closed
steveoh opened this issue Apr 26, 2022 · 16 comments · Fixed by #326
Closed

Improve secret mounting instructions #318

steveoh opened this issue Apr 26, 2022 · 16 comments · Fixed by #326
Labels
bug Something isn't working

Comments

@steveoh
Copy link

steveoh commented Apr 26, 2022

TL;DR

Following the readme was not helpful when trying to mount a secret and I had to guess multiple iterations until getting it correct.

Expected behavior

I actually expected a similar behavior to cloud run where you would do /mount/path=secretName:version|latest

Observed behavior

From the current docs...

secret_volumes: (Optional) List of key-value pairs to mount as volumes at runtime of the format "PATH=SECRET_VERSION_REF" where PATH is the mount path inside the container (e.g. "/etc/secrets/my-secret") and SECRET_VERSION_REF is a full resource name of a Google Secret Manager secret of the format "projects/p/secrets/s/versions/v". If the version is omitted, it will default to "latest".

For example, this mounts the latest value of the api-key secret at /etc/secrets/api-key inside the function's filesystem:

secret_volumes: '/etc/secrets/api-key=projects/my-project/secrets/api-key'

  1. I tried the full resource name

    • /my/mount=projects/my-projects-name/secrets/my-secret
      The secret was not mounted. no error was displayed

From some old docs in #222

  • secret_volumes: (Optional) List of key-value pairs to mount as volumes at
    runtime of the format "PATH=SECRET_VERSION_REF" where PATH is the mount path
    inside the container (e.g. "/etc/secrets/my-secret") and SECRET_VERSION_REF is
    a full resource name of a Google Secret Manager secret of the format
    "projects/p/secrets/s/versions/v". If the project is omitted, it will be
    inferred from the Cloud Function project ID. If the version is omitted, it
    will default to "latest".
  1. I tried to omit the project and the version like you can in the cloud run action
  • /my/mount=my-secret

This failed with the following and was expected since the current readme does not show these words any longer.

google-github-actions/deploy-cloud-functions failed with: failed to parse secret reference "my-secret": unknown format. Secrets should be of the format "projects/p/secrets/s/versions/v".

  1. I used the project number instead of the project name

    • /my/mount=projects/1234/secrets/my-secret

    This worked but looks different in the browser console from when I performed the same action manually in the console. It shows the link as

    my/mount projects/1234/secrets/secrets:latest
    instead of just secrets:latest. This is not a big deal but a difference I noticed.

So, I would recommend updating projects/p/secrets/s/versions/v to something like projects/123/secrets/s/versions/1 to remove any confusion that p and v are numbers and not text.

Action YAML

name: Build and Test

on:
  push:
    branches:
      - main
      - dev
  pull_request:
      branches:
      - main
      - dev

concurrency:
  group: "${{ github.head_ref || github.ref }}"
  cancel-in-progress: true

jobs:
  test:
    name: Setup and Test
    runs-on: ubuntu-latest

    steps:
    - name: Set up Python
      uses: actions/setup-python@v3
      with:
        python-version: 3.8
        # cache: pip
        # cache-dependency-path: setup.py

    - name: Checkout code
      uses: actions/checkout@v3

    - name: Install module
      run: pip install .[tests]

    - name: Test with pytest
      run: pytest

    - name: Report coverage to Codecov
      uses: codecov/codecov-action@v2
      with:
        token: ${{ secrets.CODECOV_TOKEN }}
        file: ./cov.xml

  deploy-dev:
    name: Deploy to GCF
    needs: test
    runs-on: ubuntu-latest
    if: github.ref == 'refs/heads/dev'
    environment:
      name: dev
    permissions:
      id-token: write
      contents: read

    steps:
      - name: ⬇️ Set up code
        uses: actions/checkout@v3

      - name: 🗝️ Authenticate to Google Cloud
        id: auth
        uses: google-github-actions/auth@v0
        with:
          create_credentials_file: true
          token_format: access_token
          workload_identity_provider: ${{ secrets.IDENTITY_PROVIDER }}
          service_account: ${{ secrets.SERVICE_ACCOUNT_EMAIL }}

      - name: 🚀 Deploy to Cloud Function
        id: deploy
        uses: google-github-actions/deploy-cloud-functions@v0
        with:
          name: erap-skid
          runtime: python38
          entry_point: main
          source_dir: src/erap
          service_account_email: cloud-function-sa@${{ secrets.PROJECT_ID }}.iam.gserviceaccount.com
          event_trigger_type: providers/cloud.pubsub/eventTypes/topic.publish
          event_trigger_resource: projects/${{ secrets.PROJECT_ID }}/topics/erap-scheduler
          deploy_timeout: 600
          secret_volumes: /secrets/secrets.json=${{ secrets.SECRET_MOUNT }}


      - name: 📥 Create PubSub topic
        run : |
          if [ ! "$(gcloud pubsub topics list | grep monday-morning-topic)" ]; then
            gcloud pubsub topics create monday-morning-topic --quiet
          fi

      - name: 🕰️ Create Cloud Scheduler
        run: |
          if [ ! "$(gcloud scheduler jobs list --location=us-central1 | grep monday-morning)" ]; then
            gcloud scheduler jobs create pubsub monday-morning \
              --description="Trigger the erap-skid bot once a week on monday morning" \
              --schedule="0 9 * * 1" \
              --time-zone=America/Denver \
              --location=us-central1 \
              --topic=monday-morning-topic \
              --message-body='{"run": "now"}' \
              --quiet
          else
            gcloud scheduler jobs update pubsub monday-morning \
              --description="Trigger the erap-skid bot once a week on monday morning" \
              --schedule="0 9 * * 1" \
              --time-zone=America/Denver \
              --location=us-central1 \
              --topic=monday-morning-topic \
              --message-body='{"run": "now"}' \
              --quiet
          fi

Log output

No response

Additional information

No response

@steveoh steveoh added the bug Something isn't working label Apr 26, 2022
@sethvargo
Copy link
Member

Hi @steveoh

Thank you for opening an issue. I'm not sure I understand the Cloud Run syntax of /mount/path=secretName:version|latest. From the Cloud Run docs, there's no mention of a pipe character.

The format for volume mounts is:

/path/on/disk=SECRET_VERSION

Where SECRET_VERSION can be:

  • projects/PROJECT_ID_OR_NUMBER/secrets/SECRET_NAME/versions/VERSION_NUMBER_OR_"latest"
  • projects/PROJECT_ID_OR_NUMBER/secrets/SECRET_NAME, which assumes "latest"
  • PROJECT_ID_OR_NUMBER/SECRET_NAME/VERSION_NUMBER_OR_"latest"
  • PROJECT_ID_OR_NUMBER/SECRET_NAME, which assumes "latest"

The logic for computing this is here.

@steveoh
Copy link
Author

steveoh commented Apr 26, 2022

Hey @sethvargo!

Thank you for opening an issue. I'm not sure I understand the Cloud Run syntax of /mount/path=secretName:version|latest. From the Cloud Run docs, there's no mention of a pipe character.

I'm referencing https://github.com/google-github-actions/deploy-cloudrun and I used a pipe instead of what you did with VERSION_NUMBER_OR_"latest". I like the deploy cloudrun action syntax where you only need to specify the secret name and the version. It would be nice if these two actions public api surface were consistent.

I found that using the project id was not applying the mount to the function. Only the project number added the mount. No errors were displayed but the mount was not on the function. I suppose that could be a gcloud/api issue more so than an actions issue but I think the syntax you used here would be an improvement to the current error message.

@sethvargo
Copy link
Member

Hi @steveoh

That Cloud Run syntax only applies to the gcloud CLI. The actual Cloud Run API requires a full secret version reference in the format projects/p/secrets/s/versions/v. For CLIs that's fine, but that level of "magic" in a codified CI tool like GitHub Actions could be harmful or introduce a security risk. I think it's best to require the secret's full resource name instead of inferring the project id.

The Secret Manager API accepts both project IDs and project numbers interchangeably. The API will always return project numbers in responses (even when given project IDs), but it accepts both and the most recent tests for that use case are passing. This action's integration tests also have a test and that secret uses a project ID.

It would be helpful if you could provide a minimal action.yml that demonstrates the failed mounting.

@steveoh
Copy link
Author

steveoh commented Apr 26, 2022

but that level of "magic" in a codified CI tool like GitHub Actions could be harmful or introduce a security risk

Are you saying that for the api surface of the function and run deploy actions to be similar the gcloud api would need to be consistent and building that consistency would need to happen in this action and would create the harmful security risk? Longest run on sentence ever.

Here's a sample yml file that will exercise all of the permutations that should work.

It will require a main.py, two github secrets being the PROJECT_ID = project name and PROJECT_NUMBER = project number and a secret called secrets. These should all run successfully but you will need to verify that every function has a mounted secret by looking at the variables tab for the individual functions.

name: secret mounting

on:
  workflow_dispatch:

jobs:
  deploy-dev:
    name: Deploy to GCF
    runs-on: ubuntu-latest
    permissions:
      id-token: write
      contents: read

    steps:
      - name: ⬇️ Check out code
        uses: actions/checkout@v3

      - name: 🗝️ Authenticate to Google Cloud
        uses: google-github-actions/auth@v0
        with:
          create_credentials_file: true
          token_format: access_token
          workload_identity_provider: ${{ secrets.IDENTITY_PROVIDER }}
          service_account: ${{ secrets.SERVICE_ACCOUNT_EMAIL }}

      - name: full project id with version
        uses: google-github-actions/deploy-cloud-functions@v0
        with:
          name: id-with-version
          runtime: python38
          entry_point: main
          source_dir: .
          deploy_timeout: 600
          secret_volumes: /secrets/secrets.json=projects/${{secrets.PROJECT_ID}}/secrets/secrets/versions/1

      - name: full project id with implied latest
        uses: google-github-actions/deploy-cloud-functions@v0
        with:
          name: id-with-implied-version
          runtime: python38
          entry_point: main
          source_dir: .
          deploy_timeout: 600
          secret_volumes: /secrets/secrets.json=projects/${{secrets.PROJECT_ID}}/secrets/secrets

      - name: full project number with version
        uses: google-github-actions/deploy-cloud-functions@v0
        with:
          name: number-with-version
          runtime: python38
          entry_point: main
          source_dir: .
          deploy_timeout: 600
          secret_volumes: /secrets/secrets.json=projects/${{secrets.PROJECT_NUMBER}}/secrets/secrets/versions/1

      - name: full project number with implied latest
        uses: google-github-actions/deploy-cloud-functions@v0
        with:
          name: number-with-implied-version
          runtime: python38
          entry_point: main
          source_dir: .
          deploy_timeout: 600
          secret_volumes: /secrets/secrets.json=projects/${{secrets.PROJECT_NUMBER}}/secrets/secrets

      - name: partial project id with implied latest
        uses: google-github-actions/deploy-cloud-functions@v0
        with:
          name: partial-project-with-implied-version
          runtime: python38
          entry_point: main
          source_dir: .
          deploy_timeout: 600
          secret_volumes: /secrets/secrets.json=${{secrets.PROJECT_ID}}/secrets/secrets

      - name: partial project number with implied latest
        uses: google-github-actions/deploy-cloud-functions@v0
        with:
          name: partial-number-with-implied-version
          runtime: python38
          entry_point: main
          source_dir: .
          deploy_timeout: 600
          secret_volumes: /secrets/secrets.json=${{secrets.PROJECT_NUMBER}}/secrets/secrets

I'm running it now to see for myself.

Update

so far every single one (3/6 deployed) are working as expected. Must have been a glitch in the matrix yesterday.

@sethvargo
Copy link
Member

Are you saying that for the api surface of the function and run deploy actions to be similar the gcloud api would need to be consistent and building that consistency would need to happen in this action and would create the harmful security risk? Longest run on sentence ever.

I'm saying two things:

  1. The actual API does not behave as you described. It actually accepts a proto struct and requires a full resource ID or the resource needs to be broken down into the explicit project, secret, and version components.

  2. The use cases for a CLI tool like gcloud are different than the use cases for a GitHub Actions workflow. Users are unlikely to build reproducible workflows on top of CLI commands. However, with GitHub Actions composite workflows, moving a workflow from one repo to another could change the project ID and therefore the secret(s) being accessed. This could pose a security risk if I could get you to run my workflow against your project, since it would inherit the project ID lookup for a secret.

From your update, it seems like things might be working now?

@steveoh
Copy link
Author

steveoh commented Apr 26, 2022

I'm focused on workflows and the deploy cloud run workflow does not require a full secret resource id.

for instance, this is acceptable

- name: 🚀 Deploy to Cloud Run
        id: deploy
        uses: google-github-actions/deploy-cloudrun@v0
        with:
          service: api
          image: gcr.io/${{ secrets.PROJECT_ID }}/api
          region: us-central1
          flags: |
            --service-account=cloud-run-sa@${{ secrets.PROJECT_ID }}.iam.gserviceaccount.com
          secrets: |
            /secrets/secrets.json=secrets:latest

Does this pose a security risk? Did you already tell me it doesn't? Ha, where am I?

Yes the secrets are mounting for the functions. I wonder if an IAM permission took longer to propagate.

@sethvargo
Copy link
Member

Does this pose a security risk? Did you already tell me it doesn't? Ha, where am I?

It poses a security risk if you're consuming an action like that, because the project ID would be interpreted as your project (instead of explicitly opting into that behavior).

Yes the secrets are mounting for the functions. I wonder if an IAM permission took longer to propagate.

IAM is eventually consistent, so that's possible.

@steveoh
Copy link
Author

steveoh commented Apr 27, 2022

This is happening again now after adding a new secret. Is there some extraordinarily long delay between creating a new secret and being able to mount it to a function?

@steveoh steveoh reopened this Apr 27, 2022
@steveoh
Copy link
Author

steveoh commented Apr 27, 2022

I used the same test as before and newly created functions were mounted with both secrets while the existing function kept the original secret. Are we sure that all the secrets are removed for function deployments?

All existing secret volume mounts will be removed, even if this parameter is not passed.

@sethvargo
Copy link
Member

Hi @steveoh

Without more information or a consistent reproduction case, it's difficult to diagnose.

@steveoh
Copy link
Author

steveoh commented May 10, 2022

Can you publish a new function with a secret attached successfully?

@sethvargo
Copy link
Member

@steveoh yes - our integration tests specifically do this:

secret_environment_variables: |-
FOO=${{ secrets.DEPLOY_CF_SECRET_VERSION_REF }}
BAR=${{ secrets.DEPLOY_CF_SECRET_REF }}
secret_volumes: '/etc/secrets/foo=${{ secrets.DEPLOY_CF_SECRET_VERSION_REF }}'

@steveoh
Copy link
Author

steveoh commented May 10, 2022

@sethvargo is that an overwrite of an existing function or a new function?

@sethvargo
Copy link
Member

That's a new function each time. Is it only occurring on existing functions?

@steveoh
Copy link
Author

steveoh commented May 10, 2022

I used the same test as before and newly created functions were mounted with both secrets while the existing function kept the original secret.

@sethvargo Yes it is only occurring on existing functions.

I used the same test as before and newly created functions were mounted with both secrets while the existing function kept the original secret.

You might add another test to cover this?

@sethvargo
Copy link
Member

I think I know what's going on then. I'll try to get a PR together tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants