Skip to content

Commit

Permalink
Review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
zzaakiirr committed Dec 18, 2024
1 parent 46d5c60 commit 9384ae2
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 75 deletions.
8 changes: 4 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions docs/commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/command/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions lib/command/deploy_image.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ 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
- Deploys the latest image to app workloads
- 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
Expand All @@ -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)
Expand Down
7 changes: 5 additions & 2 deletions lib/command/promote_app_from_upstream.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
4 changes: 4 additions & 0 deletions lib/core/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
4 changes: 2 additions & 2 deletions spec/command/deploy_image_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}'/)
Expand Down
126 changes: 63 additions & 63 deletions spec/command/promote_app_from_upstream_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
7 changes: 7 additions & 0 deletions spec/dummy/.controlplane/controlplane.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 9384ae2

Please sign in to comment.