From 9384ae2b04b973bc4149a81fd6542be13e82ce11 Mon Sep 17 00:00:00 2001 From: Zakir Dzhamaliddinov Date: Wed, 18 Dec 2024 14:14:18 +0300 Subject: [PATCH] Review fixes --- CHANGELOG.md | 8 +- README.md | 3 + docs/commands.md | 2 + lib/command/base.rb | 4 +- lib/command/deploy_image.rb | 5 +- lib/command/promote_app_from_upstream.rb | 7 +- lib/core/config.rb | 4 + spec/command/deploy_image_spec.rb | 4 +- .../command/promote_app_from_upstream_spec.rb | 126 +++++++++--------- spec/dummy/.controlplane/controlplane.yml | 7 + 10 files changed, 95 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d252de09..100be57a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ Changes since the last non-beta release. _Please add entries here for your pull requests that have not yet been released._ +### Added + +- Added `use-digest-image-ref` (also configurable through `use_digest_image_ref` in `controlplane.yml`) option to `deploy-image` and `promote-app-from-upstream` commands. [PR 249](https://github.com/shakacode/control-plane-flow/pull/249) by [Zakir Dzhamaliddinov](https://github.com/zzaakiirr). + ## [4.1.0] - 2024-12-17 ### Fixed @@ -28,10 +32,6 @@ _Please add entries here for your pull requests that have not yet been released. - Added `--docker-context` option to `build-image` command. [PR 250](https://github.com/shakacode/control-plane-flow/pull/250) by [Sergey Tarasov](https://github.com/dzirtusss). -### Changed - -- Providing digest (SHA256 value) for image link on promotion from upstream. [PR 249](https://github.com/shakacode/control-plane-flow/pull/249) by [Zakir Dzhamaliddinov](https://github.com/zzaakiirr). - ## [4.0.0] - 2024-08-21 ### Fixed diff --git a/README.md b/README.md index 3ced2ce0..73ba89e9 100644 --- a/README.md +++ b/README.md @@ -316,6 +316,9 @@ apps: # This is relative to the `.controlplane/` directory. release_script: release_script + # Used by the `cpflow deploy-image` and `cpflow promote-app-from-upstream` commands to include Docker image's digest (SHA256 value) in its reference. + use_digest_image_ref: true + # default_domain is used for commands that require a domain # including `maintenance`, `maintenance:on`, `maintenance:off`. default_domain: domain.com diff --git a/docs/commands.md b/docs/commands.md index a01f44e6..3ea91c84 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -127,6 +127,7 @@ cpflow delete -a $APP_NAME -w $WORKLOAD_NAME - Runs a release script before deploying if `release_script` is specified in the `.controlplane/controlplane.yml` file and `--run-release-phase` is provided - The release script is run in the context of `cpflow run` with the latest image - If the release script exits with a non-zero code, the command will stop executing and also exit with a non-zero code +- If `use_digest_image_ref` is `true` in the `.controlplane/controlplane.yml` file or `--use-digest-image-ref` option is provided, deployed image's reference will include its digest ```sh cpflow deploy-image -a $APP_NAME @@ -295,6 +296,7 @@ cpflow open-console -a $APP_NAME - Runs `cpflow deploy-image` to deploy the image - If `.controlplane/controlplane.yml` includes the `release_script`, `cpflow deploy-image` will use the `--run-release-phase` option - If the release script exits with a non-zero code, the command will stop executing and also exit with a non-zero code + - If `use_digest_image_ref` is `true` in the `.controlplane/controlplane.yml` file or `--use-digest-image-ref` option is provided, deployed image's reference will include its digest ```sh cpflow promote-app-from-upstream -a $APP_NAME -t $UPSTREAM_TOKEN diff --git a/lib/command/base.rb b/lib/command/base.rb index 9a1fe746..caac7922 100644 --- a/lib/command/base.rb +++ b/lib/command/base.rb @@ -454,9 +454,9 @@ def self.docker_context_option } end - def self.use_digest_ref_option(required: false) + def self.use_digest_image_ref_option(required: false) { - name: :use_digest_ref, + name: :use_digest_image_ref, params: { desc: "Uses the image's digest (SHA256 value) for referencing the Docker image", type: :boolean, diff --git a/lib/command/deploy_image.rb b/lib/command/deploy_image.rb index 081ed4a1..c6a3c21d 100644 --- a/lib/command/deploy_image.rb +++ b/lib/command/deploy_image.rb @@ -8,7 +8,7 @@ class DeployImage < Base OPTIONS = [ app_option(required: true), run_release_phase_option, - use_digest_ref_option + use_digest_image_ref_option ].freeze DESCRIPTION = "Deploys the latest image to app workloads, and runs a release script (optional)" LONG_DESCRIPTION = <<~DESC @@ -16,6 +16,7 @@ class DeployImage < Base - Runs a release script before deploying if `release_script` is specified in the `.controlplane/controlplane.yml` file and `--run-release-phase` is provided - The release script is run in the context of `cpflow run` with the latest image - If the release script exits with a non-zero code, the command will stop executing and also exit with a non-zero code + - If `use_digest_image_ref` is `true` in the `.controlplane/controlplane.yml` file or `--use-digest-image-ref` option is provided, deployed image's reference will include its digest DESC def call # rubocop:disable Metrics/MethodLength, Metrics/CyclomaticComplexity @@ -31,7 +32,7 @@ def call # rubocop:disable Metrics/MethodLength, Metrics/CyclomaticComplexity "Use `cpflow build-image` first." end - image = "#{image_details['name']}@#{image_details['digest']}" if config.options[:use_digest_ref] + image = "#{image_details['name']}@#{image_details['digest']}" if config.use_digest_image_ref? config[:app_workloads].each do |workload| workload_data = cp.fetch_workload!(workload) diff --git a/lib/command/promote_app_from_upstream.rb b/lib/command/promote_app_from_upstream.rb index 4e6df1f7..431f4001 100644 --- a/lib/command/promote_app_from_upstream.rb +++ b/lib/command/promote_app_from_upstream.rb @@ -5,7 +5,8 @@ class PromoteAppFromUpstream < Base NAME = "promote-app-from-upstream" OPTIONS = [ app_option(required: true), - upstream_token_option(required: true) + upstream_token_option(required: true), + use_digest_image_ref_option ].freeze DESCRIPTION = "Copies the latest image from upstream, runs a release script (optional), and deploys the image" LONG_DESCRIPTION = <<~DESC @@ -15,6 +16,7 @@ class PromoteAppFromUpstream < Base - Runs `cpflow deploy-image` to deploy the image - If `.controlplane/controlplane.yml` includes the `release_script`, `cpflow deploy-image` will use the `--run-release-phase` option - If the release script exits with a non-zero code, the command will stop executing and also exit with a non-zero code + - If `use_digest_image_ref` is `true` in the `.controlplane/controlplane.yml` file or `--use-digest-image-ref` option is provided, deployed image's reference will include its digest DESC def call @@ -32,7 +34,8 @@ def copy_image_from_upstream def deploy_image args = [] args.push("--run-release-phase") if config.current[:release_script] - run_cpflow_command("deploy-image", "-a", config.app, "--use-digest-ref", *args) + args.push("--use-digest-image-ref") if config.use_digest_image_ref? + run_cpflow_command("deploy-image", "-a", config.app, *args) end end end diff --git a/lib/core/config.rb b/lib/core/config.rb index 54d963e6..467c7145 100644 --- a/lib/core/config.rb +++ b/lib/core/config.rb @@ -145,6 +145,10 @@ def find_app_config(app_name1) end&.last end + def use_digest_image_ref? + current&.dig(:use_digest_image_ref) || options[:use_digest_image_ref] + end + private def ensure_current_config! diff --git a/spec/command/deploy_image_spec.rb b/spec/command/deploy_image_spec.rb index 1f7c0756..8878e8ea 100644 --- a/spec/command/deploy_image_spec.rb +++ b/spec/command/deploy_image_spec.rb @@ -47,11 +47,11 @@ end end - context "with --use-digest-ref option" do + context "with --use-digest-image-ref option" do let!(:app) { dummy_test_app("rails-non-app-image", create_if_not_exists: true) } it "deploys latest image with digest reference", :slow do - result = run_cpflow_command("deploy-image", "-a", app, "--use-digest-ref") + result = run_cpflow_command("deploy-image", "-a", app, "--use-digest-image-ref") expect(result[:status]).to eq(0) expect(result[:stderr]).to match(/Deploying image '#{app}:\d+@sha256:[a-fA-F0-9]{64}'/) diff --git a/spec/command/promote_app_from_upstream_spec.rb b/spec/command/promote_app_from_upstream_spec.rb index fccdaa10..4dc5c09b 100644 --- a/spec/command/promote_app_from_upstream_spec.rb +++ b/spec/command/promote_app_from_upstream_spec.rb @@ -3,60 +3,83 @@ require "spec_helper" describe Command::PromoteAppFromUpstream do - context "when release script is not provided" do - let!(:token) { Shell.cmd("cpln", "profile", "token", "default")[:output].strip } - let!(:upstream_app) { dummy_test_app } - let!(:app) { dummy_test_app("nothing") } + subject(:result) do + run_cpflow_command("promote-app-from-upstream", "-a", app, "--upstream-token", token, *extra_args) + end - before do - stub_env("CPLN_UPSTREAM", upstream_app) - # Ideally, we should have a different org, but for testing purposes, this works - stub_env("CPLN_ORG_UPSTREAM", dummy_test_org) + let(:upstream_app) { dummy_test_app } + let(:token) { Shell.cmd("cpln", "profile", "token", "default")[:output].strip } + let(:extra_args) { [] } - run_cpflow_command!("apply-template", "app", "-a", upstream_app) - run_cpflow_command!("apply-template", "app", "rails", "-a", app) - run_cpflow_command!("build-image", "-a", upstream_app) - end + before do + stub_env("CPLN_UPSTREAM", upstream_app) + # Ideally, we should have a different org, but for testing purposes, this works + stub_env("CPLN_ORG_UPSTREAM", dummy_test_org) + stub_env("APP_NAME", app) - after do - run_cpflow_command!("delete", "-a", upstream_app, "--yes") - run_cpflow_command!("delete", "-a", app, "--yes") - end + run_cpflow_command!("apply-template", "app", "-a", upstream_app) + run_cpflow_command!("apply-template", "app", "rails", "postgres", "-a", app) + run_cpflow_command!("build-image", "-a", upstream_app) + end - it "copies latest image from upstream, skips release script and deploys image", :slow do - result = run_cpflow_command("promote-app-from-upstream", "-a", app, "--upstream-token", token) + after do + run_cpflow_command!("delete", "-a", upstream_app, "--yes") + run_cpflow_command!("delete", "-a", app, "--yes") + end + shared_examples "copies latest image from upstream and deploys image" do |**options| + it "#{options[:runs_release_script] ? 'runs' : 'does not run'} release script", :slow do expect(result[:status]).to eq(0) expect(result[:stderr]).to match(%r{Pulling image from '.+?/#{upstream_app}:1'}) expect(result[:stderr]).to match(%r{Pushing image to '.+?/#{app}:1'}) - expect(result[:stderr]).not_to include("Running release script") - expect(result[:stderr]).to match(/Deploying image '#{app}:1@sha256:[a-fA-F0-9]{64}'/) + + if options[:runs_release_script] + expect(result[:stderr]).to include("Running release script") + else + expect(result[:stderr]).not_to include("Running release script") + end + + if options[:uses_digest_image_ref] + expect(result[:stderr]).to match(/Deploying image '#{app}:1@sha256:[a-fA-F0-9]{64}'/) + else + expect(result[:stderr]).to match(/Deploying image '#{app}:1(?!@)'/) + end + expect(result[:stderr]).to match(%r{rails: https://rails-.+?.cpln.app}) end end + context "when release script is not provided" do + let(:app) { dummy_test_app("nothing") } + + it_behaves_like "copies latest image from upstream and deploys image", + runs_release_script: false, + uses_digest_image_ref: false + + context "with use_digest_image_ref from YAML file" do + let(:app) { dummy_test_app("use-digest-image-ref") } + + it_behaves_like "copies latest image from upstream and deploys image", + runs_release_script: false, + uses_digest_image_ref: true + end + + context "with --use-digest-image-ref option" do + let(:extra_args) { ["--use-digest-image-ref"] } + + it_behaves_like "copies latest image from upstream and deploys image", + runs_release_script: false, + uses_digest_image_ref: true + end + end + context "when release script is invalid" do - let!(:token) { Shell.cmd("cpln", "profile", "token", "default")[:output].strip } - let!(:upstream_app) { dummy_test_app } - let!(:app) { dummy_test_app("invalid-release-script") } + let(:app) { dummy_test_app("invalid-release-script") } before do - stub_env("CPLN_UPSTREAM", upstream_app) - # Ideally, we should have a different org, but for testing purposes, this works - stub_env("CPLN_ORG_UPSTREAM", dummy_test_org) - stub_env("APP_NAME", app) - - run_cpflow_command!("apply-template", "app", "-a", upstream_app) - run_cpflow_command!("apply-template", "app", "rails", "postgres", "-a", app) - run_cpflow_command!("build-image", "-a", upstream_app) run_cpflow_command!("ps:start", "-a", app, "--workload", "postgres", "--wait") end - after do - run_cpflow_command!("delete", "-a", upstream_app, "--yes") - run_cpflow_command!("delete", "-a", app, "--yes") - end - it "copies latest image from upstream, fails to run release script and fails to deploy image", :slow do result = run_cpflow_command("promote-app-from-upstream", "-a", app, "--upstream-token", token) @@ -71,37 +94,14 @@ end context "when release script is valid" do - let!(:token) { Shell.cmd("cpln", "profile", "token", "default")[:output].strip } - let!(:upstream_app) { dummy_test_app } - let!(:app) { dummy_test_app } + let(:app) { dummy_test_app } before do - stub_env("CPLN_UPSTREAM", upstream_app) - # Ideally, we should have a different org, but for testing purposes, this works - stub_env("CPLN_ORG_UPSTREAM", dummy_test_org) - stub_env("APP_NAME", app) - - run_cpflow_command!("apply-template", "app", "-a", upstream_app) - run_cpflow_command!("apply-template", "app", "rails", "postgres", "-a", app) - run_cpflow_command!("build-image", "-a", upstream_app) run_cpflow_command!("ps:start", "-a", app, "--workload", "postgres", "--wait") end - after do - run_cpflow_command!("delete", "-a", upstream_app, "--yes") - run_cpflow_command!("delete", "-a", app, "--yes") - end - - it "copies latest image from upstream, runs release script and deploys image", :slow do - result = run_cpflow_command("promote-app-from-upstream", "-a", app, "--upstream-token", token) - - expect(result[:status]).to eq(0) - expect(result[:stderr]).to match(%r{Pulling image from '.+?/#{upstream_app}:1'}) - expect(result[:stderr]).to match(%r{Pushing image to '.+?/#{app}:1'}) - expect(result[:stderr]).to include("Running release script") - expect(result[:stderr]).to include("Finished running release script") - expect(result[:stderr]).to match(/Deploying image '#{app}:1@sha256:[a-fA-F0-9]{64}'/) - expect(result[:stderr]).to match(%r{rails: https://rails-.+?.cpln.app}) - end + it_behaves_like "copies latest image from upstream and deploys image", + runs_release_script: true, + uses_digest_image_ref: false end end diff --git a/spec/dummy/.controlplane/controlplane.yml b/spec/dummy/.controlplane/controlplane.yml index 01d1c997..261658be 100644 --- a/spec/dummy/.controlplane/controlplane.yml +++ b/spec/dummy/.controlplane/controlplane.yml @@ -133,6 +133,13 @@ apps: upstream: dummy-test-upstream release_script: release-invalid.sh + dummy-test-use-digest-image-ref-{GLOBAL_IDENTIFIER}: + <<: *common + + match_if_app_name_starts_with: true + upstream: dummy-test-upstream + use_digest_image_ref: true + dummy-test-external-maintenance-image-{GLOBAL_IDENTIFIER}: <<: *common