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

Also build for arm64 #914

Merged
merged 3 commits into from
Mar 13, 2024
Merged

Also build for arm64 #914

merged 3 commits into from
Mar 13, 2024

Conversation

jksolbakken
Copy link
Collaborator

@jksolbakken jksolbakken commented Mar 12, 2024

Summary:
Builds Docker images also for ARM. Improves developer experience on newer Macs with ARM CPU (Mx)
Adds necessary changes to the affected GitHub workflows

Description for the changelog:
add build for arm64 docker image in GitHub workflows

Other info:
The extra build time should be bearable, if not the builds for each arch can be run in parallel using a matrix.

Copy link
Collaborator

@jgadsden jgadsden left a comment

Choose a reason for hiding this comment

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

superb, many thanks @jksolbakken

@jksolbakken
Copy link
Collaborator Author

The build fails because of missing secrets for login to Docker Hub. Maybe remove the login step and do not push from the PR workflow?

@jgadsden
Copy link
Collaborator

possible - although the e2e and zap stages may fail if this is done? Looking at it again the workflow should be changed anyway to cache the docker image so that e2e and zap work OK
if you want to experiment please go ahead, I will give you some permissions to run the workflows @jksolbakken

@jksolbakken
Copy link
Collaborator Author

I will update this PR to run the build step before the login for PRs. That way we can determine if the image build itself works without the need for elevated privs. The other steps should work as before when you merge. Secrets are not available if the workflow was caused by an external PR.

@jksolbakken
Copy link
Collaborator Author

No, wait that won't work. I'll come up with a solution.

@jgadsden jgadsden self-requested a review March 13, 2024 09:12
Copy link
Collaborator

@jgadsden jgadsden left a comment

Choose a reason for hiding this comment

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

thanks agin @jksolbakken , and will go ahead and merge

@jgadsden jgadsden merged commit cf30109 into OWASP:main Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request version-2.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants