Add GITHUB_AUTH_TOKEN as arg to Dockerfile to RUN commands using phive

[bdovaz]

Context: #2312 (comment)

[nvuillam]

I think that we should stop maintaining the shell files calling docker and use docker-build github action, like discussed in this thread -> #2256

[bdovaz]

@nvuillam please read the following message carefully so that you understand how this PR makes sense regardless of whether we continue using upload-docker.sh or not please.... I have tried in several messages to make you understand but either I don't explain myself well or I don't know :(

#2312 (comment)

[nvuillam]

That is, we have to modify the descriptor to pass that environment variable to each phive command. Example:

Line 133 in b0bb50f

      RUN phive --no-progress install psalm -g --trust-gpg-keys 8A03EA3B385DBAA1,12CE0F1D262429A5

I think I understand that, and I agree, I just would prefer that GITHUB_TOKEN was sent through docker-build standard action, so we could do some same for all other linters also requiring it ^^

[nvuillam]

🦙 MegaLinter status: ⚠️ WARNING

Descriptor	Linter	Files	Fixed	Errors	Elapsed time
✅ BASH	bash-exec	6		0	0.01s
✅ BASH	shellcheck	6		0	0.16s
✅ BASH	shfmt	6	0	0	0.41s
✅ COPYPASTE	jscpd	yes		no	3.76s
✅ DOCKERFILE	hadolint	105		0	12.01s
✅ JSON	eslint-plugin-jsonc	21	0	0	2.7s
✅ JSON	jsonlint	19		0	0.37s
✅ JSON	v8r	21		0	15.36s
⚠️ MARKDOWN	markdownlint	309	0	231	7.48s
✅ MARKDOWN	markdown-link-check	309		0	6.19s
✅ MARKDOWN	markdown-table-formatter	309	0	0	20.71s
✅ OPENAPI	spectral	1		0	2.12s
⚠️ PYTHON	bandit	176		45	2.54s
✅ PYTHON	black	176	0	0	4.74s
✅ PYTHON	flake8	176		0	2.17s
✅ PYTHON	isort	176	0	0	1.04s
✅ PYTHON	mypy	176		0	9.16s
✅ PYTHON	pylint	176		0	14.87s
⚠️ PYTHON	pyright	176		276	22.58s
✅ REPOSITORY	checkov	yes		no	35.75s
✅ REPOSITORY	git_diff	yes		no	0.41s
✅ REPOSITORY	secretlint	yes		no	16.53s
✅ REPOSITORY	trivy	yes		no	31.95s
✅ SPELL	cspell	729		0	26.31s
✅ SPELL	misspell	550	0	0	0.99s
✅ XML	xmllint	3	0	0	0.46s
✅ YAML	prettier	81	0	0	3.55s
✅ YAML	v8r	23		0	72.28s
✅ YAML	yamllint	82		0	1.26s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by

[bdovaz]

@nvuillam as I say, to solve the problem that this PR pretends to solve it is not relevant if it is done with the bash script or with the action that you mention...

To get to the point that you comment it is necessary to rewrite all the logic of upload-docker.sh to use that action instead of the bash script and that is a titanic task and that requires a lot of precision because that script is very big and has many conditions, etc...

That must be done with time.... Besides that it is going to require that at least 2 people review it because of the regressions that it can have.

I don't know if @echoix feels up to the task that would benefit us in #2273.

[echoix]

(Read the following as a code review, not bashing 😜)
I think that there's a little better way to implement what you have just added, as what I understand, it might appear in the image's history.

First of all, I think that the variable should be "GITHUB_TOKEN", as that's what I think you're using to authenticate. It'll be easier to follow along knowing that GITHUB_AUTH_TOKEN is in reality GITHUB_TOKEN, where there is more help on the web.

Then, here is some docs https://docs.docker.com/build/ci/github-actions/examples/#secrets and https://docs.docker.com/engine/reference/commandline/buildx_build/#secret
We can use the secrets feature of docker building. Look also at https://stackoverflow.com/questions/68275321/how-to-use-github-token-in-dockerfile-that-is-built-in-github-actions-and-trying, where in the question his usage is right.
I'm not sure, but maybe the syntax dockerfile won't be needed (see the second link, in the env section).

I need to go, without finishing my message, but it's a good start

[codecov-commenter]

Codecov Report

Merging #2314 (788a70f) into main (b0bb50f) will increase coverage by 0.02%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #2314      +/-   ##
==========================================
+ Coverage   82.88%   82.90%   +0.02%     
==========================================
  Files         171      171              
  Lines        4523     4523              
==========================================
+ Hits         3749     3750       +1     
+ Misses        774      773       -1     

Impacted Files	Coverage Δ
megalinter/reporters/UpdatedSourcesReporter.py	89.74% <0.00%> (+2.56%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[nvuillam]

🦙 MegaLinter status: ⚠️ WARNING

Descriptor	Linter	Files	Fixed	Errors	Elapsed time
✅ BASH	bash-exec	6		0	0.01s
✅ BASH	shellcheck	6		0	0.15s
✅ BASH	shfmt	6	0	0	0.04s
✅ COPYPASTE	jscpd	yes		no	3.0s
✅ DOCKERFILE	hadolint	105		0	8.26s
✅ JSON	eslint-plugin-jsonc	21	0	0	1.82s
✅ JSON	jsonlint	19		0	0.24s
✅ JSON	npm-package-json-lint	yes		no	0.68s
✅ JSON	v8r	21		0	11.8s
⚠️ MARKDOWN	markdownlint	309	2	231	5.96s
✅ MARKDOWN	markdown-link-check	309		0	5.38s
✅ MARKDOWN	markdown-table-formatter	309	2	0	16.76s
✅ OPENAPI	spectral	1		0	1.67s
⚠️ PYTHON	bandit	176		45	1.94s
✅ PYTHON	black	176	0	0	3.08s
✅ PYTHON	flake8	176		0	1.73s
✅ PYTHON	isort	176	0	0	0.43s
✅ PYTHON	mypy	176		0	7.06s
✅ PYTHON	pylint	176		0	11.07s
⚠️ PYTHON	pyright	176		274	17.19s
✅ REPOSITORY	checkov	yes		no	27.53s
⚠️ REPOSITORY	devskim	yes		61	1.24s
✅ REPOSITORY	dustilock	yes		no	1.79s
✅ REPOSITORY	git_diff	yes		no	0.03s
✅ REPOSITORY	secretlint	yes		no	8.16s
✅ REPOSITORY	syft	yes		no	0.91s
✅ REPOSITORY	trivy	yes		no	16.58s
✅ SPELL	cspell	729		0	17.67s
✅ SPELL	misspell	550	2	0	0.55s
✅ XML	xmllint	3	0	0	0.03s
✅ YAML	prettier	81	0	0	2.52s
✅ YAML	v8r	23		0	53.68s
✅ YAML	yamllint	82		0	1.17s

See detailed report in MegaLinter reports

You could have same capabilities but better runtime performances if you request a new MegaLinter flavor.

• Click here to request the new flavor

MegaLinter is graciously provided by

[echoix]

@bdovaz See how they add it in Super-Linter : https://github.com/github/super-linter/blob/13e5f8b29a34bbf7ce2476292477fa385bec2578/scripts/install-phive.sh#L6-L15

[bdovaz]

@echoix about the variable name, it is not a mistake, it is called that way in PHIVE:

https://github.com/phar-io/phive/blob/c5985a10f95be9a21b79e7ee19bef39dc85e9f95/src/shared/config/EnvironmentAuthConfig.php#L18

And about the rest, you are right I understand that it would serve with a:

RUN export GITHUB_AUTH_TOKEN="$(cat /run/secrets/GITHUB_TOKEN)" && phive

That's what you mean, isn't it?

But it is true that for that both this PR and #2299 depend on rewriting all workflows to remove the bash script from upload-docker.sh and use the native actions for Docker.

[nvuillam]

I don't think that using docker action is so complicated to migrate (because it basically does the same than what we do, and our bash scripts are SuperLinter ones, and they migrated ),

So I can probably try to do that next week ^^

[echoix]

I don't think that only by using docker build action the GITHUB_TOKEN would be used everywhere for every call inside the build environnement (building the containers), we will still need to add them in the generated Dockerfiles

[echoix]

@bdovaz ok, if PHIVE needs it called that way, fine, but everywhere else that value is called GITHUB_TOKEN, and the value that we will pass it will be called GITHUB_TOKEN, including what GitHub will give us.