Migrate to github actions #50

thomasferrandiz · 2024-03-13T10:32:41Z

github-advanced-security · 2024-03-13T11:13:11Z

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

manuelbuil · 2024-03-13T14:13:16Z

.github/workflows/build.yml

+        tags: rancher/hardened-coredns:${{ env.TAG }}-arm64
+        file: Dockerfile
+        outputs: type=docker
+        platforms: linux/arm64


I wonder if it makes sense to also run trivy here, maybe there are extra stuff when using arm64 code?

I'm not sure if it's necessary or not.
I remember that we didn't do it for s390x but maybe for arm64 it's more useful.

Not sure there is much value on per-arch Trivy scan, I would assume the majority of times you will get the same (or very similar results).

For reference, atm all our internal scans are solely on amd64.

Agree, if the packages in the final images per arch are the same, you could only do the scan in amd64. Although, it shouldn't be a big issue if you scan all of them, it's more a way of saving time in the pipeline.

pjbgf · 2024-03-13T17:08:40Z

.github/workflows/build.yml

+    - name: Build container image
+      uses: docker/build-push-action@v5
+      with:


Any specific reason to break the builds per platform? One of the key benefits of docker buildx is optimised cross-compilation builds.

@pjbgf I think it might be because of this issue: docker/buildx#59

The default image store in Docker Engine doesn't support loading multi-platform images. You can enable the containerd image store, or push multi-platform images is to directly push to a registry

using --load and --platform at the same time in docker buildx is not supported

yes exactly @manuelbuil did some tests for this PR: rancher/image-build-flannel#76 and concluded the build per platform was needed

If we are solely testing the build (e.g. confirm whether it works for all target platforms), you can remove both --load and --push and set --platform. That effectively means you will build to all target platforms. Like we did here.

.github/workflows/build.yml

.github/workflows/image-push.yml

briandowns · 2024-03-13T17:19:11Z

@galal-hussein is doing this for a lot of the other, non networking related repositories. Let's make sure we're doing all of them as similar as possible.

.github/workflows/build.yml

galal-hussein · 2024-03-13T19:02:19Z

lets also remove the .drone part from updatecli as well https://github.com/rancher/image-build-coredns/blob/master/updatecli/updatecli.d/updatebuildbase.yaml#L45-L53

pjbgf · 2024-03-13T19:16:52Z

The drone webhooks should also be removed - probably one for EIO.

macedogm

Nice PR! Everything looks good. The main recommendation is to follow the least privilege when generating the token, as Paulo suggested.

macedogm · 2024-03-14T10:27:45Z

.github/workflows/build.yml

+        tags: rancher/hardened-coredns:${{ env.TAG }}-arm64
+        file: Dockerfile
+        outputs: type=docker
+        platforms: linux/arm64


Agree, if the packages in the final images per arch are the same, you could only do the scan in amd64. Although, it shouldn't be a big issue if you scan all of them, it's more a way of saving time in the pipeline.

macedogm · 2024-03-14T10:33:22Z

Makefile

@@ -53,3 +49,14 @@ image-manifest:
 .PHONY: image-scan
 image-scan:


Should this target also be removed?

pjbgf · 2024-03-14T15:47:42Z

Please note that the host-arch is taking 2min while the cross arch is taking 14min. We could get this optimised by letting the heavy lifting be done at the build arch instead of cross emulating network operations. However that could be dealt with on a follow-up PR.

manuelbuil · 2024-03-18T15:10:34Z

@pjbgf @macedogm Insisting on Hussein's question: do you think it is a good idea to upload the output: 'trivy-results.sarif' in all the repos?

macedogm · 2024-03-18T15:28:00Z

@pjbgf @macedogm Insisting on Hussein's question: do you think it is a good idea to upload the output: 'trivy-results.sarif' in all the repos?

Yes, although we already have such info in our internal image-scanning, having it per repo can speed up specific analysis and visibility by the teams themselves.

manuelbuil · 2024-03-18T16:20:19Z

@pjbgf @macedogm Insisting on Hussein's question: do you think it is a good idea to upload the output: 'trivy-results.sarif' in all the repos?

Yes, although we already have such info in our internal image-scanning, having it per repo can speed up specific analysis and visibility by the teams themselves.

Thanks! I'm trying to write down all taken decisions in this migration so that we can replicate them in other repos :)

macedogm · 2024-03-19T12:07:25Z

Thanks! I'm trying to write down all taken decisions in this migration so that we can replicate them in other repos :)

@manuelbuil Nice! We can help review the doc and add specific guidance if needed.

manuelbuil · 2024-03-19T12:12:40Z

Thanks! I'm trying to write down all taken decisions in this migration so that we can replicate them in other repos :)

@manuelbuil Nice! We can help review the doc and add specific guidance if needed.

It's in the description of this EPIC: rancher/ecm-distro-tools#375

Signed-off-by: Thomas Ferrandiz <[email protected]>

thomasferrandiz · 2024-04-10T13:15:32Z

replaced by #52

thomasferrandiz · 2024-04-11T12:25:46Z

reference issue: rancher/ecm-distro-tools#382

thomasferrandiz force-pushed the migrateToGHA branch from 9014675 to 40d201c Compare March 13, 2024 11:10

manuelbuil reviewed Mar 13, 2024

View reviewed changes

thomasferrandiz requested review from galal-hussein, macedogm and tashima42 March 13, 2024 15:17

pjbgf reviewed Mar 13, 2024

View reviewed changes

.github/workflows/build.yml Show resolved Hide resolved

pjbgf reviewed Mar 13, 2024

View reviewed changes

.github/workflows/image-push.yml Outdated Show resolved Hide resolved

galal-hussein reviewed Mar 13, 2024

View reviewed changes

.github/workflows/build.yml Show resolved Hide resolved

macedogm reviewed Mar 14, 2024

View reviewed changes

Makefile

@@ -53,3 +49,14 @@ image-manifest:

.PHONY: image-scan

image-scan:

Copy link

Member

macedogm Mar 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this target also be removed?

macedogm approved these changes Mar 14, 2024

View reviewed changes

pjbgf approved these changes Mar 14, 2024

View reviewed changes

thomasferrandiz force-pushed the migrateToGHA branch 2 times, most recently from d3d163f to 7bb2cbc Compare March 14, 2024 16:59

thomasferrandiz force-pushed the migrateToGHA branch from 7bb2cbc to cf06453 Compare March 19, 2024 09:03

Migrate to github actions

2adc200

Signed-off-by: Thomas Ferrandiz <[email protected]>

thomasferrandiz force-pushed the migrateToGHA branch from cf06453 to 2adc200 Compare March 21, 2024 10:06

thomasferrandiz mentioned this pull request Apr 2, 2024

optimize build time by using cross-compilation instead of emulation #52

Merged

thomasferrandiz closed this Apr 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to github actions #50

Migrate to github actions #50

thomasferrandiz commented Mar 13, 2024 •

edited

Loading

github-advanced-security bot commented Mar 13, 2024

manuelbuil Mar 13, 2024

thomasferrandiz Mar 13, 2024

pjbgf Mar 13, 2024

macedogm Mar 14, 2024

pjbgf Mar 13, 2024

tashima42 Mar 13, 2024 •

edited

Loading

thomasferrandiz Mar 13, 2024

pjbgf Mar 13, 2024

briandowns commented Mar 13, 2024

galal-hussein commented Mar 13, 2024

pjbgf commented Mar 13, 2024

macedogm left a comment

macedogm Mar 14, 2024

macedogm Mar 14, 2024

pjbgf commented Mar 14, 2024

manuelbuil commented Mar 18, 2024

macedogm commented Mar 18, 2024

manuelbuil commented Mar 18, 2024

macedogm commented Mar 19, 2024

manuelbuil commented Mar 19, 2024

thomasferrandiz commented Apr 10, 2024

thomasferrandiz commented Apr 11, 2024

		@@ -53,3 +49,14 @@ image-manifest:
		.PHONY: image-scan
		image-scan:

Migrate to github actions #50

Migrate to github actions #50

Conversation

thomasferrandiz commented Mar 13, 2024 • edited Loading

github-advanced-security bot commented Mar 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tashima42 Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

briandowns commented Mar 13, 2024

galal-hussein commented Mar 13, 2024

pjbgf commented Mar 13, 2024

macedogm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjbgf commented Mar 14, 2024

manuelbuil commented Mar 18, 2024

macedogm commented Mar 18, 2024

manuelbuil commented Mar 18, 2024

macedogm commented Mar 19, 2024

manuelbuil commented Mar 19, 2024

thomasferrandiz commented Apr 10, 2024

thomasferrandiz commented Apr 11, 2024

thomasferrandiz commented Mar 13, 2024 •

edited

Loading

tashima42 Mar 13, 2024 •

edited

Loading