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

[match] prevent directory download from s3 #20975

Merged

Conversation

markhomoki
Copy link
Contributor

@markhomoki markhomoki commented Jan 6, 2023

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I see several green ci/circleci builds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

Currently, we use match and store our certificates on GitHub. However, we decided to move away from GitHub and store the certificates on s3. There are several iOS apps we maintain. After migrating the certificates to s3 (and following the same format match uses), we faced a problem when match tries to download the root folder instead of the certificates.
Resolves #19363

Description

When match creates the distribution certificate and pushes it to s3, everything works perfectly. However, when uploading the exact files and folder structure to s3, the match script cannot find the certificates the script fails. It happens because in that cases, s3_client.find_bucket!(s3_bucket).objects(prefix: s3_object_prefix) returns the root directory as well, which fails with the following error: No such file or directory @ rb_sysopen. Adding a check to prevent downloading directories, fixes the problem.

Testing Steps

  1. Let match create the certificates and upload them to s3
  2. Login to s3 where you can access files
  3. Make a note of the team name, then rename the folder to something else
  4. Change the folder name back to the team name
  5. Without this fix, the match script fails

@google-cla
Copy link

google-cla bot commented Jan 6, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@markhomoki markhomoki changed the title fix: prevent directory download from s3 [match] prevent directory download from s3 Mar 1, 2023
Copy link

@mikiest mikiest left a comment

Choose a reason for hiding this comment

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

Seems very reasonable and low risk.

@markhomoki
Copy link
Contributor Author

@rogerluan Sorry for tagging you, but I'm curious if this can be merged, or there's anything I need to update. Unfortunately, we are unable to use the latest version of fastlane without this PR.

@rogerluan
Copy link
Member

Hi @markhomoki !

Thanks for opening this PR 🙏 Any chance you can add unit tests that cover this case? That way we can ensure this works as expected, and that it won't regress in the future as well

Improvements

* [core][match] remove obsolete and expired WWDR G1 certificate (fastlane#21271) via Frederik Seiffert (@triplef)
* [action][ensure_git_status_clean] new ignore_files option for explicitly ignoring files (fastlane#21283) via Josh Holtz (@joshdholtz)
* [scan] run simulator destination with arch=x86_64 for Xcode 14.3 and up if on Intel (fastlane#21284) via Josh Holtz (@joshdholtz)
* [match] adding support for self-managed GitLab instances (fastlane#21274) via Darby Frey (@darbyfrey)
* [pilot] fix increase limit for build query (fastlane#21212) via Eric Lindvall (@eric)
* [dependency] relax `multipart_post` dependency version requirement (fastlane#20870) via Edouard Brière (@edouard)

https://github.com/fastlane/fastlane/releases/tag/2.213.0
@markhomoki
Copy link
Contributor Author

markhomoki commented Aug 14, 2023

Thanks @rogerluan. Sorry for taking this long, but i've updated my PR. Tested the cases locally and they are working. For some reason it fails on Xcode 13.0.0 with Ruby 3.1.. (Also, seen other PRs that have the same issue)

@rogerluan
Copy link
Member

Thanks for the heads up @markhomoki , I'll check what's going on 💪

@rogerluan
Copy link
Member

This might fix CI, let's see: #21465

@markhomoki
Copy link
Contributor Author

Thanks @rogerluan , seems like all good

match/lib/match/storage/s3_storage.rb Outdated Show resolved Hide resolved
Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>
Copy link
Member

@rogerluan rogerluan left a comment

Choose a reason for hiding this comment

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

💪 Looks good! Thank you for this PR! 🤗

@rogerluan rogerluan merged commit ab9fb32 into fastlane:master Aug 30, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is a directory @ rb_sysopen - error
3 participants