Missing Env Variables

[sprankhub]

As part of #69, we encountered that there are quite some input variables, which are not passed into the Docker container. https://github.com/extdn/github-actions-m2/blob/4091e3ae8e47b4fdaa049a9c64a088c1d06d02f1/magento-integration-tests/entrypoint.sh uses the following inputs, which are not passed into the Docker container here:

github-actions-m2/magento-integration-tests/8.1/action.yml

Lines 44 to 49 in 4091e3a

	env:
	MODULE_NAME: ${{ inputs.module_name }}
	COMPOSER_NAME: ${{ inputs.composer_name }}
	MAGENTO_VERSION: ${{ inputs.ce_version }}
	PROJECT_NAME: ${{ inputs.project_name }}
	COMPOSER_MEMORY_LIMIT: -1

• INPUT_ELASTICSEARCH
• INPUT_PHPUNIT_FILE
• INPUT_COMPOSER_VERSION
• INPUT_REPOSITORY_URL
• INPUT_PRE_PROJECT_SCRIPT
• INPUT_POST_PROJECT_SCRIPT
• INPUT_MAGENTO_PRE_INSTALL_SCRIPT
• INPUT_MAGENTO_POST_INSTALL_SCRIPT

Wouldn't all these variables need to be passed as well? Am I missing something?

/cc @jissereitsma

[jissereitsma]

Hmm, I understand that earlier PR #69 better now. The thing is that both inputs and env variables are supposed to be supported in your own workflow YAML. For instance, if I would create the following workflow (for static tests, but it illustrates the idea):

on: [push]
jobs:
  static:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - uses: extdn/github-actions-m2/magento-coding-standard/8.1@master
        with:
          phpcs_severity: 6

then it would equal the following:

on: [push]
jobs:
  static:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - uses: extdn/github-actions-m2/magento-coding-standard/8.1@master
        env:
          PHPCS_SEVERITY: 6

which again equals:

name: ExtDN Static Tests
on: [push]
env:
  PHPCS_SEVERITY: 6
jobs:
  static:
    name: Static Code Analysis
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - uses: extdn/github-actions-m2/magento-coding-standard/8.1@master

In other words, either you would define an input foobar (using with in your own workflow YAML) which translates within the GitHub Action of ExtDN to INPUT_FOOBAR or you would define an env FOOBAR. And next, the GitHub Action of ExtDN, simply sees if FOOBAR is empty and then uses INPUT_FOOBAR as an alternative. At least this is what the original scripts set up by @fooman did and I always just copied things from it.

Following from this, in the Integration Test scripts, the variable ELASTICSEARCH for instance is usable via env (

github-actions-m2/magento-integration-tests/entrypoint.sh

Line 11 in 4091e3a

test -z "${ELASTICSEARCH}" && ELASTICSEARCH=$INPUT_ELASTICSEARCH

). But (!) because its definition is missing in the action.yml, people might not know about this. Plus, because the variable is not defined as input in action.yml, the variable is never allowed as input either. Therefore the INPUT_ELASTICSEARCH variable will never do anything: This is incomplete and should be fixed.

As I see it, there are 2 issues:

1. All input variables that are mentioned in the entrypoint.sh script, should also be mentioned in action.yml (complete with comments and optional flag) under input. This allows better documenting of what is done. This needs to be done for variables like ELASTICSEARCH (input elasticsearch) and so on.

2. Currently, users might have implemented either env or with in their own workflow YAML. And because of this, I think that both should work. But this actually means that the action.yml should not copy the input variable into an env variable (like ELASTICSEARCH: ${{ inputs.elasticsearch }}) as suggested by @BorisovskiP . This will break the workflow of people that are now using the env variables to configure things (I'm using both). This would require that commit
   96abbdf is undone.

[sprankhub]

1. Fully agree.
2. Hmm not sure if I follow. How would that work then? Where do variables like $INPUT_MODULE_NAME actually come from in entrypoint.sh? At a certain point, we need to make it consistent and need to decide what takes precedence: Env variables or input variables. This might be a one-time breaking change for some images.

[jissereitsma]

Let me illustrate with another variable phpunit_file.

This is defined in https://github.com/extdn/github-actions-m2/blob/master/magento-integration-tests/8.1/action.yml#L22 as a valid input, though not required. This means that in a workflow YAML you can use this via with, referring to the variable phpunit_file (lower case). However, such an input variable is used in the entrypoint bash script as INPUT_PHPUNIT_FILE (so uppercased and prefixed with INPUT_).

This conversion of the variable name happens with inputs, but not with env variables. In other words, if I would define an env variable in my workflow YAML called PHPUNIT_FILE, this would still be that same variable name PHPUNIT_FILE in the entrypoint bash script as well.

So, depending on the type of variable you define in your workflow (input or env), the variable name either becomes INPUT_PHPUNIT_FILE or PHPUNIT_FILE. And that if-else is picked up by the entrypoint script: https://github.com/extdn/github-actions-m2/blob/master/magento-integration-tests/entrypoint.sh#L12

[sprankhub]

So the input phpunit_file is indeed automatically converted into an env variable INPUT_PHPUNIT_FILE by GitHub Actions?

I'd say we just need to define what takes precedence and then change this in all places, so that it is the same everywhere. What do you think makes more sense? I do not really care - mixing both approaches (so using input variables and env variables) does not make sense to me anyway.

[jissereitsma]

So the input phpunit_file is indeed automatically converted into an env variable INPUT_PHPUNIT_FILE by GitHub Actions?
Yes indeed.

So far so good, all is working. And I really would like to keep both options (env and secret) in there, because otherwise existing flows might be broken.

But let's keep this ticket open until I found the time to make sure that all possible variables are usable in this way.