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: encrypt artifacts #2567

Merged
merged 78 commits into from
Dec 20, 2023
Merged

ci: encrypt artifacts #2567

merged 78 commits into from
Dec 20, 2023

Conversation

miampf
Copy link
Contributor

@miampf miampf commented Nov 8, 2023

Context

Currently, any artifacts uploaded by the CI are unencrypted which can be insecure (e.g. for logs).

Proposed change(s)

  • add the action .github/actions/artifact_upload
  • add the action .github/actions/artifact_download
  • replace all CI references to actions/upload-artifact/actions/download-artifact with their respective wrappers

Checklist

  • Add labels (e.g., for changelog category)
  • Is PR title adequate for changelog?

@miampf miampf added security fix Vulnerability fixes no changelog Change won't be listed in release changelog labels Nov 8, 2023
Copy link

netlify bot commented Nov 8, 2023

Deploy Preview for constellation-docs canceled.

Name Link
🔨 Latest commit 506398d
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/6582f1e9f8d6430007f4bde0

@malt3 malt3 requested a review from msanft November 8, 2023 15:31
@msanft msanft removed the security fix Vulnerability fixes label Nov 8, 2023
@msanft
Copy link
Contributor

msanft commented Nov 8, 2023

Removed the security-fix label as this is rather a defensive security measure on our side and since it interferes with the no-changelog label in the changelog generation process

Copy link
Contributor

@msanft msanft left a comment

Choose a reason for hiding this comment

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

One general thing: If making as many commits is part of your standard workflow, awesome! If you do this because you think we expect it. No. Feel free to do less commits, don't feel urged by the "guidelines" saying that you should push your work regularly. This is really regular. However, I really don't wan't you to change it if it's part of your default workflow. Just don't feel urged to do that by us :D

.github/actions/artifact_download/action.yml Outdated Show resolved Hide resolved
.github/actions/artifact_download/action.yml Outdated Show resolved Hide resolved
.github/actions/artifact_upload/action.yml Outdated Show resolved Hide resolved
.github/actions/artifact_upload/action.yml Outdated Show resolved Hide resolved
.github/actions/artifact_download/action.yml Outdated Show resolved Hide resolved
.github/actions/artifact_download/action.yml Outdated Show resolved Hide resolved
.github/actions/artifact_upload/action.yml Outdated Show resolved Hide resolved
@miampf miampf force-pushed the encrypt-artifacts branch 6 times, most recently from 4f03a7b to dc86d93 Compare November 15, 2023 11:08
Copy link
Contributor

@msanft msanft left a comment

Choose a reason for hiding this comment

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

Changes look really good to me already other than that :)

.github/actions/artifact_upload/action.yml Outdated Show resolved Hide resolved
.github/actions/artifact_download/action.yml Show resolved Hide resolved
.github/actions/artifact_upload/action.yml Outdated Show resolved Hide resolved
.github/actions/e2e_benchmark/action.yml Show resolved Hide resolved
.github/workflows/e2e-upgrade.yml Show resolved Hide resolved
.github/workflows/release-cli.yml Outdated Show resolved Hide resolved
.github/workflows/release-cli.yml Outdated Show resolved Hide resolved
.github/workflows/test-artifact-actions-and-stuff.yml Outdated Show resolved Hide resolved
test-folder/file-1.txt Outdated Show resolved Hide resolved
.github/workflows/test-artifact-actions-and-stuff.yml Outdated Show resolved Hide resolved
.github/actions/artifact_upload/action.yml Outdated Show resolved Hide resolved
.github/actions/artifact_upload/action.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@msanft msanft left a comment

Choose a reason for hiding this comment

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

LGTM except for the test removal that Paul mentioned.

@miampf miampf requested a review from katexochen November 22, 2023 11:26
@miampf miampf force-pushed the encrypt-artifacts branch 2 times, most recently from 2489a80 to a088b9b Compare November 27, 2023 13:04
@msanft msanft requested a review from malt3 November 28, 2023 08:05
Copy link
Contributor

@malt3 malt3 left a comment

Choose a reason for hiding this comment

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

LGTM

@malt3
Copy link
Contributor

malt3 commented Dec 18, 2023

I think this can be merged 🚀

.github/actions/constellation_create/action.yml Outdated Show resolved Hide resolved
.github/workflows/draft-release.yml Outdated Show resolved Hide resolved
.github/workflows/draft-release.yml Show resolved Hide resolved
@miampf miampf merged commit a429ca5 into main Dec 20, 2023
5 checks passed
@miampf miampf deleted the encrypt-artifacts branch December 20, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog Change won't be listed in release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants