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

testing: Add support for runtime environment variables to GitHub Workflows #3501

Open
grayside opened this issue Sep 12, 2023 · 3 comments
Open
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. samples Issues that are directly related to samples. type: cleanup An internal cleanup or hygiene concern.

Comments

@grayside
Copy link
Collaborator

GitHub Workflows is the preferred test driver for tests in this repository, but the current workflow implementation has several limitations. One of them is the ability to set runtime environment variables that are specific to a Workflow.

Approach

Options:

  1. Hard code a central map of environment variables, and refactor code to ensure clean namespacing
  2. Create a mechanism to pass environment variables to our reusable test workflow

Option 2 is the most versatile with the least need to refactor existing code.

Option 1 has the further downside of placing test-specific parameters in a central file. I'm not sure if any changes to test.yml are triggering wide-scale test execution currently, but with this approach we'd likely need to trigger more tests from any change, which would include adding environment variables for unrelated tests.

@pattishin has asked that as part of the PR fixing this issue, we look for existing hard-coded values and shift them to environment variable configuration.

This issue is a blocker on implementation of GHA-based test workflows for a number of samples, with #3149 as the driver for research today.

@grayside grayside added type: cleanup An internal cleanup or hygiene concern. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Sep 12, 2023
@grayside grayside self-assigned this Sep 12, 2023
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Sep 12, 2023
@grayside
Copy link
Collaborator Author

Here are some notes towards Option 2 from the OP:

Illustrating the thought just a bit further (this is a thought experiment, not tested!)

in functions-v2-imagemagick.yaml:

Add a new job:

  setup:
    runs-on: ubuntu-latest
    permissions: {}
    steps:
    - run: |
        echo "FUNCTIONS_BUCKET=nodejs-docs-samples-tests" >> $GITHUB_OUTPUT
        echo "BLURRED_BUCKET_NAME=nodejs-docs-samples-tests-imagick" >> $GITHUB_OUTPUT
        cat $GITHUB_OUTPUT

Add use of this to the test job:

  test:
    # Ref: https://github.com/google-github-actions/auth#usage
    permissions:
      contents: 'read'
      id-token: 'write'
    if: github.event.action != 'labeled' || github.event.label.name == 'actions:force-run'
    uses: ./.github/workflows/test.yaml
    with:
      name: 'functions-v2-imagemagick'
      path: 'functions/v2/imagemagick'
      env: needs.setup.outputs
    needs: [setup]

In test.yaml add a new input:

      env:
        type: string

Then after the authentication step, load the environment from input to env:

    - name: Set caller env
      run: |
        echo "${{inputs.env}}" >> $GITHUB_ENV

If some variant of this works, we now have a way for the calling workflow to pass unexpected environment variables to the test workflow.

@kweinmeister
Copy link
Collaborator

Just FYI, we may not be blocked, as this capability already exists in the workflow generator, with several samples using this workflow. This ticket could cover potential refactorings if needed.

@grayside
Copy link
Collaborator Author

Ah, thank you for pointing this out. I saw a couple examples of the samples using the workflow-secrets template but I interpreted them as one-off forks of the reusable test.yml.

I don't have access to review the existing secrets, but there is notionally a limit of 100 per repo: https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions#limits-for-secrets.

This opens three tracks for this issue:

  1. Close the issue, we have the capability and are not concerned with the limitation
  2. Refactor how we use secrets because the limitation is a concern (e.g., a single secret per product that holds a set of values)
  3. Switch to something like the model in this issue (if it works) and limit secrets to configurations with security concerns

The advantage of the approach proposed in this issue is it helps reduce structural variation in the individual workflows, meaning we may have more flexibility in the future to group tests or apply matrix strategies.

I'm happy to move forward with #3149 via the workflow-secrets template in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. samples Issues that are directly related to samples. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

No branches or pull requests

3 participants