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

Fix issue that doesn't allow using upstream with match_if_app_name_starts_with set to true in copy-image-from-upstream command #136

Merged
merged 5 commits into from
Jan 22, 2024
+9 −8
Merged

Fix issue that doesn't allow using upstream with match_if_app_name_starts_with set to true in copy-image-from-upstream command #136

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
04c8876
fix: ensure that profile is switched back to default
rafaelgomesxyz Jan 19, 2024
9deb4ec
docs: update CHANGELOG
rafaelgomesxyz Jan 19, 2024
765ebfd
fix: allow using upstream that has match_if_app_name_starts_with set …
rafaelgomesxyz Jan 19, 2024
d1fd881
docs: update CHANGELOG
rafaelgomesxyz Jan 19, 2024
3e0a38f
Merge branch 'main' into copy-image-from-upstream-starts-with-fix
justin808 Jan 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ Changes since the last non-beta release.

_Please add entries here for your pull requests that are not yet released._

### Fixed

- Fixed issue where cpln profile was not switched back to `default` if an error happened while running `copy-image-from-upstream` command. [PR 135](https://github.com/shakacode/heroku-to-control-plane/pull/135) by [Rafael Gomes](https://github.com/rafaelgomesxyz).
- Fixed issue that didn't allow using upstream with `match_if_app_name_starts_with` set to `true` in `copy-image-from-upstream` command. [PR 136](https://github.com/shakacode/heroku-to-control-plane/pull/136) by [Rafael Gomes](https://github.com/rafaelgomesxyz).

### Added

- Added `--domain` option to `maintenance`, `maintenance:on` and `maintenance:off` commands. [PR 131](https://github.com/shakacode/heroku-to-control-plane/pull/131) by [Rafael Gomes](https://github.com/rafaelgomesxyz).
Expand Down
3 changes: 2 additions & 1 deletion lib/command/copy_image_from_upstream.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def call # rubocop:disable Metrics/MethodLength
ensure_docker_running!

@upstream = config[:upstream]
@upstream_org = config.apps[@upstream.to_sym][:cpln_org] || ENV.fetch("CPLN_ORG_UPSTREAM", nil)
@upstream_org = config.find_app_config(@upstream)&.dig(:cpln_org) || ENV.fetch("CPLN_ORG_UPSTREAM", nil)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's say we have a dummy-test app in controlplane.yml that has match_if_app_name_starts_with set to true.

The old method wouldn't work if called with, e.g., dummy-test-123, because config.apps only contains the apps exactly as they are in controlplane.yml, and we have dummy-test in controlplane.yml, not dummy-test-123.

The new method works by iterating through config.apps looking for an app that matches dummy-test-123, so it will find dummy-test and match it.

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why we'd use a starts with for the upstream. That should be a validation error. Agree, @dzirtusss

ensure_upstream_org!

create_upstream_profile
Expand All @@ -38,6 +38,7 @@ def call # rubocop:disable Metrics/MethodLength
pull_image_from_upstream
push_image_to_app
ensure
cp.profile_switch("default")
delete_upstream_profile
end

Expand Down
14 changes: 7 additions & 7 deletions lib/core/config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,13 @@ def current
end
end

def find_app_config(app_name1)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just made it public to be able to use it.

@app_configs ||= {}
@app_configs[app_name1] ||= apps.find do |app_name2, app_config|
app_matches?(app_name1, app_name2, app_config)
end&.last
end

private

def ensure_current_config!
Expand All @@ -127,13 +134,6 @@ def app_matches?(app_name1, app_name2, app_options)
)
end

def find_app_config(app_name1)
@app_configs ||= {}
@app_configs[app_name1] ||= apps.find do |app_name2, app_config|
app_matches?(app_name1, app_name2, app_config)
end&.last
end

def ensure_app!
return if app

Expand Down