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.

Sign up for GitHub

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

Jump to bottom

[match] Added s3_skip_encryption parameter #21018

Merged
merged 14 commits into from
Sep 4, 2023
Merged

[match] Added s3_skip_encryption parameter #21018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
d7c7871
Added s3_skip_encryption parameter to match
mbogh Jan 23, 2023
0ea04c4
Fixed tests for now.
mbogh Jan 30, 2023
001b67d
Added tests for encryption for_storage_mode
mbogh Feb 7, 2023
7af388b
Forgot rubocop
mbogh Feb 7, 2023
52507e5
Added warning to option
mbogh Mar 14, 2023
fd31159
for_mode to from_params
mbogh Mar 14, 2023
f0f1e84
Merge branch 'master' into match-skip-encryption
getaaron Aug 27, 2023
fd8a503
Fix GitLabSecureFiles config params
getaaron Aug 27, 2023
b82a1a9
disable ClassLength rubocop on match/options.rb
getaaron Aug 27, 2023
a24fbc7
Remove no longer needed fields from importer spec assertion
getaaron Aug 27, 2023
2179f5e
Remove trailing comma
getaaron Aug 27, 2023
63b23ac
Removed one more param from importer_spec
getaaron Aug 27, 2023
4813d3d
A bit of clean up should make the tests go green
mbogh Sep 4, 2023
6f7b56d
One comma
mbogh Sep 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 3 additions & 9 deletions match/lib/match/change_password.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,13 @@ def self.update(params: nil)
new_password = FastlaneCore::Helper.ask_password(message: "New passphrase for Git Repo: ", confirm: true)

# Choose the right storage and encryption implementations
storage = Storage.for_mode(params[:storage_mode], {
git_url: params[:git_url],
shallow_clone: params[:shallow_clone],
skip_docs: params[:skip_docs],
git_branch: params[:git_branch],
git_full_name: params[:git_full_name],
git_user_email: params[:git_user_email],
clone_branch_directly: params[:clone_branch_directly]
})
storage = Storage.from_params(params)
storage.download

encryption = Encryption.for_storage_mode(params[:storage_mode], {
git_url: params[:git_url],
s3_bucket: params[:s3_bucket],
s3_skip_encryption: params[:s3_skip_encryption],
working_directory: storage.working_directory
})
encryption.decrypt_files
Expand Down
9 changes: 3 additions & 6 deletions match/lib/match/commands_generator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,13 @@ def run
params = FastlaneCore::Configuration.create(Match::Options.available_options, options.__hash__)
params.load_configuration_file("Matchfile")

storage = Storage.for_mode(params[:storage_mode], {
git_url: params[:git_url],
shallow_clone: params[:shallow_clone],
git_branch: params[:git_branch],
clone_branch_directly: params[:clone_branch_directly]
})
storage = Storage.from_params(params)
storage.download

encryption = Encryption.for_storage_mode(params[:storage_mode], {
git_url: params[:git_url],
s3_bucket: params[:s3_bucket],
s3_skip_encryption: params[:s3_skip_encryption],
working_directory: storage.working_directory
})
encryption.decrypt_files if encryption
Expand Down
2 changes: 1 addition & 1 deletion match/lib/match/encryption.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def backends
},
"s3" => lambda { |params|
params[:keychain_name] = params[:s3_bucket]
return Encryption::OpenSSL.configure(params)
return params[:s3_skip_encryption] ? nil : Encryption::OpenSSL.configure(params)
},
"gitlab_secure_files" => lambda { |params|
return nil
Expand Down
34 changes: 3 additions & 31 deletions match/lib/match/importer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,42 +15,14 @@ def import_cert(params, cert_path: nil, p12_path: nil, profile_path: nil)
profile_path = ensure_valid_file_path(profile_path, "Provisioning profile", ".mobileprovision or .provisionprofile", optional: true)

# Storage
storage = Storage.for_mode(params[:storage_mode], {
git_url: params[:git_url],
shallow_clone: params[:shallow_clone],
skip_docs: params[:skip_docs],
git_branch: params[:git_branch],
git_full_name: params[:git_full_name],
git_user_email: params[:git_user_email],
git_private_key: params[:git_private_key],
git_basic_authorization: params[:git_basic_authorization],
git_bearer_authorization: params[:git_bearer_authorization],
clone_branch_directly: params[:clone_branch_directly],
type: params[:type].to_s,
platform: params[:platform].to_s,
google_cloud_bucket_name: params[:google_cloud_bucket_name].to_s,
google_cloud_keys_file: params[:google_cloud_keys_file].to_s,
google_cloud_project_id: params[:google_cloud_project_id].to_s,
skip_google_cloud_account_confirmation: params[:skip_google_cloud_account_confirmation],
s3_bucket: params[:s3_bucket],
s3_region: params[:s3_region],
s3_access_key: params[:s3_access_key],
s3_secret_access_key: params[:s3_secret_access_key],
s3_object_prefix: params[:s3_object_prefix],
gitlab_project: params[:gitlab_project],
gitlab_host: params[:gitlab_host],
readonly: params[:readonly],
username: params[:username],
team_id: params[:team_id],
team_name: params[:team_name],
api_key_path: params[:api_key_path],
api_key: params[:api_key]
})
storage = Storage.from_params(params)
storage.download

# Encryption
encryption = Encryption.for_storage_mode(params[:storage_mode], {
git_url: params[:git_url],
s3_bucket: params[:s3_bucket],
s3_skip_encryption: params[:s3_skip_encryption],
working_directory: storage.working_directory
})
encryption.decrypt_files if encryption
Expand Down
8 changes: 6 additions & 2 deletions match/lib/match/migrate.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ def migrate(params)

# We init the Google storage client before the git client
# to ask for all the missing inputs *before* cloning the git repo
google_cloud_storage = Storage.for_mode("google_cloud", {
google_cloud_storage = Storage.from_params({
storage_mode: "google_cloud",
google_cloud_bucket_name: params[:google_cloud_bucket_name],
google_cloud_keys_file: params[:google_cloud_keys_file],
google_cloud_project_id: params[:google_cloud_project_id]
})

git_storage = Storage.for_mode("git", {
git_storage = Storage.from_params({
storage_mode: "git",
git_url: params[:git_url],
shallow_clone: params[:shallow_clone],
git_branch: params[:git_branch],
Expand All @@ -29,6 +31,8 @@ def migrate(params)

encryption = Encryption.for_storage_mode(params[:storage_mode], {
git_url: params[:git_url],
s3_bucket: params[:s3_bucket],
s3_skip_encryption: params[:s3_skip_encryption],
working_directory: git_storage.working_directory
})
encryption.decrypt_files if encryption
Expand Down
28 changes: 3 additions & 25 deletions match/lib/match/nuke.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,36 +34,14 @@ def run(params, type: nil)

spaceship_login

self.storage = Storage.for_mode(params[:storage_mode], {
git_url: params[:git_url],
shallow_clone: params[:shallow_clone],
skip_docs: params[:skip_docs],
git_branch: params[:git_branch],
git_full_name: params[:git_full_name],
git_user_email: params[:git_user_email],

git_private_key: params[:git_private_key],
git_basic_authorization: params[:git_basic_authorization],
git_bearer_authorization: params[:git_bearer_authorization],

clone_branch_directly: params[:clone_branch_directly],
google_cloud_bucket_name: params[:google_cloud_bucket_name].to_s,
google_cloud_keys_file: params[:google_cloud_keys_file].to_s,
google_cloud_project_id: params[:google_cloud_project_id].to_s,
s3_region: params[:s3_region].to_s,
s3_access_key: params[:s3_access_key].to_s,
s3_secret_access_key: params[:s3_secret_access_key].to_s,
s3_bucket: params[:s3_bucket].to_s,
s3_object_prefix: params[:s3_object_prefix].to_s,
gitlab_project: params[:gitlab_project],
gitlab_host: params[:gitlab_host],
team_id: params[:team_id] || Spaceship::ConnectAPI.client.portal_team_id
})
self.storage = Storage.from_params(params)
self.storage.download

# After the download was complete
self.encryption = Encryption.for_storage_mode(params[:storage_mode], {
git_url: params[:git_url],
s3_bucket: params[:s3_bucket],
s3_skip_encryption: params[:s3_skip_encryption],
working_directory: storage.working_directory
})
self.encryption.decrypt_files if self.encryption
Expand Down
7 changes: 7 additions & 0 deletions match/lib/match/options.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require_relative 'module'

module Match
# rubocop:disable Metrics/ClassLength
class Options
# This is match specific, as users can append storage specific options
def self.append_option(option)
Expand Down Expand Up @@ -222,6 +223,11 @@ def self.available_options
env_name: "MATCH_S3_OBJECT_PREFIX",
description: "Prefix to be used on all objects uploaded to S3",
optional: true),
FastlaneCore::ConfigItem.new(key: :s3_skip_encryption,
env_name: "MATCH_S3_SKIP_ENCRYPTION",
description: "Skip encryption of all objects uploaded to S3. WARNING: only enable this on S3 buckets with sufficiently restricted permissions and server-side encryption enabled. See https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingEncryption.html",
type: Boolean,
default_value: false),

# Storage: GitLab Secure Files
FastlaneCore::ConfigItem.new(key: :gitlab_project,
Expand Down Expand Up @@ -349,4 +355,5 @@ def self.available_options
]
end
end
# rubocop:enable Metrics/ClassLength
end
37 changes: 5 additions & 32 deletions match/lib/match/runner.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,43 +31,16 @@ def run(params)
update_optional_values_depending_on_storage_type(params)

# Choose the right storage and encryption implementations
self.storage = Storage.for_mode(params[:storage_mode], {
git_url: params[:git_url],
shallow_clone: params[:shallow_clone],
skip_docs: params[:skip_docs],
git_branch: params[:git_branch],
git_full_name: params[:git_full_name],
git_user_email: params[:git_user_email],
clone_branch_directly: params[:clone_branch_directly],
git_basic_authorization: params[:git_basic_authorization],
git_bearer_authorization: params[:git_bearer_authorization],
git_private_key: params[:git_private_key],
type: params[:type].to_s,
generate_apple_certs: params[:generate_apple_certs],
platform: params[:platform].to_s,
google_cloud_bucket_name: params[:google_cloud_bucket_name].to_s,
google_cloud_keys_file: params[:google_cloud_keys_file].to_s,
google_cloud_project_id: params[:google_cloud_project_id].to_s,
skip_google_cloud_account_confirmation: params[:skip_google_cloud_account_confirmation],
s3_region: params[:s3_region],
s3_access_key: params[:s3_access_key],
s3_secret_access_key: params[:s3_secret_access_key],
s3_bucket: params[:s3_bucket],
s3_object_prefix: params[:s3_object_prefix],
gitlab_project: params[:gitlab_project],
gitlab_host: params[:gitlab_host],
readonly: params[:readonly],
username: params[:readonly] ? nil : params[:username], # only pass username if not readonly
team_id: params[:team_id],
team_name: params[:team_name],
api_key_path: params[:api_key_path],
api_key: params[:api_key]
})
storage_params = params
storage_params[:username] = params[:readonly] ? nil : params[:username] # only pass username if not readonly
self.storage = Storage.from_params(storage_params)
storage.download

# Init the encryption only after the `storage.download` was called to have the right working directory
encryption = Encryption.for_storage_mode(params[:storage_mode], {
git_url: params[:git_url],
s3_bucket: params[:s3_bucket],
s3_skip_encryption: params[:s3_skip_encryption],
working_directory: storage.working_directory
})
encryption.decrypt_files if encryption
Expand Down
2 changes: 1 addition & 1 deletion match/lib/match/setup.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def run(path, is_swift_fastfile: false)
self.storage_options
)

storage = Storage.for_mode(storage_mode, {})
storage = Storage.from_params({ storage_mode: storage_mode })

specific_content = storage.generate_matchfile_content
UI.crash!("Looks like `generate_matchfile_content` was `nil` for `#{storage_mode}`") if specific_content.nil?
Expand Down
61 changes: 56 additions & 5 deletions match/lib/match/storage.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,66 @@ class << self
def backends
@backends ||= {
"git" => lambda { |params|
return Storage::GitStorage.configure(params)
return Storage::GitStorage.configure({
type: params[:type],
platform: params[:platform],
git_url: params[:git_url],
shallow_clone: params[:shallow_clone],
skip_docs: params[:skip_docs],
git_branch: params[:git_branch],
git_full_name: params[:git_full_name],
git_user_email: params[:git_user_email],
clone_branch_directly: params[:clone_branch_directly],
git_basic_authorization: params[:git_basic_authorization],
git_bearer_authorization: params[:git_bearer_authorization],
git_private_key: params[:git_private_key]
})
},
"google_cloud" => lambda { |params|
return Storage::GoogleCloudStorage.configure(params)
return Storage::GoogleCloudStorage.configure({
type: params[:type],
platform: params[:platform],
google_cloud_bucket_name: params[:google_cloud_bucket_name],
google_cloud_keys_file: params[:google_cloud_keys_file],
google_cloud_project_id: params[:google_cloud_project_id],
readonly: params[:readonly],
username: params[:username],
team_id: params[:team_id],
team_name: params[:team_name],
api_key_path: params[:api_key_path],
api_key: params[:api_key],
skip_google_cloud_account_confirmation: params[:skip_google_cloud_account_confirmation]
})
},
"s3" => lambda { |params|
return Storage::S3Storage.configure(params)
return Storage::S3Storage.configure({
s3_region: params[:s3_region],
s3_access_key: params[:s3_access_key],
s3_secret_access_key: params[:s3_secret_access_key],
s3_bucket: params[:s3_bucket],
s3_object_prefix: params[:s3_object_prefix],
readonly: params[:readonly],
username: params[:username],
team_id: params[:team_id],
team_name: params[:team_name],
api_key_path: params[:api_key_path],
api_key: params[:api_key]
})
},
"gitlab_secure_files" => lambda { |params|
return Storage::GitLabSecureFiles.configure(params)
return Storage::GitLabSecureFiles.configure({
gitlab_host: params[:gitlab_host],
gitlab_project: params[:gitlab_project],
git_url: params[:git_url], # enables warning about unnecessary git_url
job_token: params[:job_token],
private_token: params[:private_token],
readonly: params[:readonly],
username: params[:username],
team_id: params[:team_id],
team_name: params[:team_name],
api_key_path: params[:api_key_path],
api_key: params[:api_key]
})
}
}
end
Expand All @@ -39,7 +89,8 @@ def register_backend(type: nil, storage_class: nil, &configurator)
end
end

def for_mode(storage_mode, params)
def from_params(params)
Copy link
Member

Choose a reason for hiding this comment

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

It seems like this change wasn't made backwards compatible 😬😬
This was brought to my attention in fastlane/docs#1225

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there anything I can do to help?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is really a public API. The case in the docs (in the other PR) is a pretty niche use case / workaround for missing match functionality so I think this PR is probably fine. If anything we could add a test for the use case in the docs so we'd notice if we need to update them.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah fair enough. Not sure if we have any guidelines on what to consider important to keep backwards compatible and what not to, so that's to blame haha
Whenever I personally make changes or review code I try to keep all APIs backwards compatible, but that may be exclusive to me and not really the team's preferences

I'll kick off a discussion around this in Slack so perhaps we can better define this :)

Thanks for the input guys!

storage_mode = params[:storage_mode]
configurator = backends[storage_mode.to_s]
return configurator.call(params) if configurator

Expand Down
7 changes: 6 additions & 1 deletion match/spec/change_password_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@
git_branch: "master",
git_full_name: nil,
git_user_email: nil,
clone_branch_directly: false
clone_branch_directly: false,
git_basic_authorization: nil,
git_bearer_authorization: nil,
git_private_key: nil,
type: config[:type],
platform: config[:platform]
).and_return(fake_storage)

allow(fake_storage).to receive(:download)
Expand Down
14 changes: 13 additions & 1 deletion match/spec/commands_generator_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,18 @@ def expect_githelper_clone_with(git_url, shallow_clone, git_branch)
expect(Match::Storage::GitStorage).to receive(:configure).with({
git_url: git_url,
shallow_clone: shallow_clone,
skip_docs: false,
git_branch: git_branch[:branch],
clone_branch_directly: git_branch[:clone_branch_directly]
clone_branch_directly: git_branch[:clone_branch_directly],
git_full_name: nil,
git_user_email: nil,

git_private_key: nil,
git_basic_authorization: nil,
git_bearer_authorization: nil,

type: "development",
platform: "ios"
}).and_return(fake_storage)

expect(fake_storage).to receive(:download)
Expand All @@ -89,6 +99,8 @@ def expect_githelper_clone_with(git_url, shallow_clone, git_branch)

expect(Match::Encryption).to receive(:for_storage_mode).with("git", {
git_url: git_url,
s3_bucket: nil,
s3_skip_encryption: false,
working_directory: fake_working_directory
}).and_return(fake_encryption)

Expand Down