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

ci: sign all image builds #1691

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

arthurus-rex
Copy link
Contributor

This builds on previous commit 9d567e5 to add cryptographic signing to two additional workflows: image_pr.yml and image_base.yml. This resolves the issue with images only being intermittently signed. It also reduces the scope of privileges on image.yml to only what is necessary for the OIDC token to be used.

Copy link
Contributor

github-actions bot commented Aug 12, 2024

🤖 SeineSailor

Here is a concise summary of the pull request changes:

Error Handling and Code Consistency: Replaced hardcoded error strings with a variable, improved error handling, and updated the creation of error variables. Modified the ComponentModelWeights.String() function to return a formatted string.

Code Organization and Maintainability: Renamed variables to follow a consistent naming convention, introduced helper functions, and updated Go dependencies to improve project security and maintainability.

Image Security and Compression: Introduced cryptographic signing to two additional workflows, reduced privileges in the image.yml workflow, and modified eBPF code, bash script, and Go code to accommodate these changes. Improved compression format implementation, added new functions and interfaces, and introduced changes to the huff0 and snapref packages.

Build and Deployment: Introduced a new build package, modified the AppConfig struct, and updated the main function to use newAppConfig(). Replaced versioning-related variables with build.Version, build.Branch, etc.

Observations and Suggestions:

  • The pull request includes a wide range of changes, which may make it difficult to review and test thoroughly. It's recommended to break down the changes into smaller, more focused pull requests in the future.
  • The introduction of cryptographic signing and compression improvements will enhance image security and project maintainability.
  • The changes to the build package and AppConfig struct may affect the external interface and behavior of the code, so thorough testing is recommended to ensure no regressions.
  • The consistent naming convention and code organization improvements will make the codebase more maintainable and easier to understand.

@arthurus-rex
Copy link
Contributor Author

I can see that one of the integration tests here failed, but at a glance, it doesn't seem to me like this failure is related to the changes I've applied to this PR. If it is, then I will gladly make any necessary changes.

@rootfs
Copy link
Contributor

rootfs commented Aug 13, 2024

CI failure potentially addressed in #1686

@arthurus-rex
Copy link
Contributor Author

Got it. I will incorporate changes once fix is finalized.

@rootfs
Copy link
Contributor

rootfs commented Aug 15, 2024

@arthurus-rex can you rebase to pick up the CI fix?

@arthurus-rex
Copy link
Contributor Author

Successfully rebased!

@arthurus-rex arthurus-rex force-pushed the main branch 2 times, most recently from 6c9b7eb to b2ed71d Compare August 20, 2024 12:08
Add cosign to two additional image building workflows to resolve
intermittent signing issue.  Uses GitHub OIDC token for signing,
and narrows scope of privileges to only what is necessary to
write token.

Signed-off-by: Arthur Savage <[email protected]>
@dave-tucker dave-tucker merged commit 8bcbc25 into sustainable-computing-io:main Aug 22, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants