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

Sanitise repo names used as zip file names in AWS uploads #58

Closed
wants to merge 1 commit into from

Conversation

mfitz
Copy link
Contributor

@mfitz mfitz commented Nov 27, 2024

@D-Dulius and @Dominic-Duke: I've added you both as reviewers because you've recently expressed an interest in learning more about GitHub actions and workflows. Don't feel the need to formally review the PR unless you want to.

Verification

I used the version of the AWS upload action from this branch in a branch of BitSim that I am working on to replace inline code in the CI build with our reusable actions.

I confirmed that the zip file uploaded to S3 from the CI build on that branch now uses a lowercase name and that it now triggers the downstream AWS CD pipeline as a result:

aws s3 ls s3://<code-bucket-name-elided>/ | grep -i bitsim | sort -r

2024-11-26 16:14:34     851080 bitsim.zip # zip file uploaded by the branch version of the S3 upload action
2024-11-26 16:05:35     851163 BitSim.zip # zip file uploaded by the current main version of the S3 upload action

Alternative Approaches

It's somewhat opaque to transform the repo name inside this action. I can't think of an obvious problem with that for our typical use cases, but perhaps some use cases will require the repo name to remain in its original format for some reason.

Another approach would be to add an optional param to the action to override the repo/zip file name if present and default to the current behaviour where no param is supplied. It would then be up to workflow calling the action to transform the repo name as appropriate before asking for the repo to be uploaded to S3.

That would be cleaner in some ways, but it's slightly more work and might be overkill. Let me know what you think.

Copy link

@KasiaKoz KasiaKoz left a comment

Choose a reason for hiding this comment

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

it's does feel weird to change the name of the repo, but it's fine, we won't name two different tools bitsim and BitSim so there won't be any clashes... 👍

@mfitz
Copy link
Contributor Author

mfitz commented Nov 27, 2024

it's does feel weird to change the name of the repo, but it's fine, we won't name two different tools bitsim and BitSim so there won't be any clashes... 👍

Well, what I'm thinking about is users of this action from outside CML. Somebody may have a repo with a mixed-case name, and they may want to preserve that mixed-case name in the zip file uploaded to S3. Maybe they have something downstream of S3 that requires the mixed-case name (s3 names are case-sensitive and you cannot change that).

The more I think about it, I'm talking myself into the optional param approach. I think I'm going to do that. It should be up to the caller to specify the name they want for the zip file; the default will be the repo name in its exact GitHub format, and if you don't want that, you provide the name you do want.

@mfitz
Copy link
Contributor Author

mfitz commented Nov 27, 2024

Closing this as I've decided on a different approach. Rubber ducking for the win!

@mfitz mfitz closed this Nov 27, 2024
@mfitz mfitz deleted the make-s3-uploads-use-lower-case-repo-names branch November 27, 2024 14:50
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.

Make the zip file name an optional param in aws-upload.yml
2 participants