From b41b31c8ebe0105b13bb3805808fd0ea5112c9fd Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Tue, 2 Feb 2021 13:33:02 -0500 Subject: [PATCH 01/27] initial attempt --- lib/krane/deploy_task.rb | 46 +- lib/krane/kubernetes_resource.rb | 5 + lib/krane/resource_deployer.rb | 1 - .../mutating_webhooks/ingress_hook.yml | 42 + test/integration-serial/serial_deploy_test.rb | 1021 +++++++++-------- .../serial_task_run_test.rb | 126 +- 6 files changed, 662 insertions(+), 579 deletions(-) create mode 100644 test/fixtures/mutating_webhooks/ingress_hook.yml diff --git a/lib/krane/deploy_task.rb b/lib/krane/deploy_task.rb index aa60189e0..52e117658 100644 --- a/lib/krane/deploy_task.rb +++ b/lib/krane/deploy_task.rb @@ -277,16 +277,46 @@ def validate_configuration(prune:) end measure_method(:validate_configuration) + def partition_dry_run_resources(resources) + individuals = [] + out, err, st = kubectl.run("get", "mutatingwebhookconfigurations", "-ojson", + use_namespace: false, raise_if_not_found: true) + raise FatalDeploymentError, "error retrieving mutatingwebhookconfigurations: #{err.message}" unless st.success? + + mutating_webhooks = JSON.parse(out).dig("items") + mutating_webhooks.each do |spec| + spec.dig('webhooks').each do |webhook| + match_policy = webhook.dig('matchPolicy') + webhook.dig('rules').each do |rule| + next if %w(None NoneOnDryRun).include?(rule.dig('sideEffects')) + groups = rule.dig('apiGroups') + versions = rule.dig('apiVersions') + kinds = rule.dig('resources').map(&:singularize) + groups.each do |group| + versions.each do |version| + kinds.each do |kind| + individuals += resources.select do |r| + (r.group == group || group == '*' || match_policy == "Equivalent") && + (r.version == version || version == '*' || match_policy == "Equivalent") && + (r.type.downcase == kind.downcase ) + end + resources -= individuals + end + end + end + end + end + end + [resources, individuals] + end + def validate_resources(resources) validate_globals(resources) - batch_dry_run_success = kubectl.server_dry_run_enabled? && validate_dry_run(resources) - Krane::Concurrency.split_across_threads(resources) do |r| - # No need to pass in kubectl (and do per-resource dry run apply) if batch dry run succeeded - if batch_dry_run_success - r.validate_definition(kubectl: nil, selector: @selector, dry_run: false) - else - r.validate_definition(kubectl: kubectl, selector: @selector, dry_run: true) - end + batchable_resources, individuals = partition_dry_run_resources(resources) + batch_dry_run_success = kubectl.server_dry_run_enabled? && validate_dry_run(batchable_resources) + individuals += batchable_resources unless batch_dry_run_success + Krane::Concurrency.split_across_threads(individuals) do |r| + r.validate_definition(kubectl: kubectl, selector: @selector, dry_run: true) end failed_resources = resources.select(&:validation_failed?) if failed_resources.present? diff --git a/lib/krane/kubernetes_resource.rb b/lib/krane/kubernetes_resource.rb index 2335a8f38..c68d685c5 100644 --- a/lib/krane/kubernetes_resource.rb +++ b/lib/krane/kubernetes_resource.rb @@ -226,6 +226,11 @@ def group version ? grouping : "core" end + def version + prefix, version = @definition.dig("apiVersion").split("/") + version ? version : prefix + end + def kubectl_resource_type type end diff --git a/lib/krane/resource_deployer.rb b/lib/krane/resource_deployer.rb index 569356692..5efbb5b35 100644 --- a/lib/krane/resource_deployer.rb +++ b/lib/krane/resource_deployer.rb @@ -161,7 +161,6 @@ def apply_all(resources, prune, dry_run: false) global_mode = resources.all?(&:global?) out, err, st = kubectl.run(*command, log_failure: false, output_is_sensitive: output_is_sensitive, attempts: 2, use_namespace: !global_mode) - tags = statsd_tags + (dry_run ? ['dry_run:true'] : ['dry_run:false']) Krane::StatsD.client.distribution('apply_all.duration', Krane::StatsD.duration(start), tags: tags) if st.success? diff --git a/test/fixtures/mutating_webhooks/ingress_hook.yml b/test/fixtures/mutating_webhooks/ingress_hook.yml new file mode 100644 index 000000000..a629dccba --- /dev/null +++ b/test/fixtures/mutating_webhooks/ingress_hook.yml @@ -0,0 +1,42 @@ +apiVersion: v1 +items: +- apiVersion: admissionregistration.k8s.io/v1 + kind: MutatingWebhookConfiguration + metadata: + name: ingress-webhook-configuration + webhooks: + - admissionReviewVersions: + - v1beta1 + clientConfig: + caBundle: BUNDLE + service: + name: boss + namespace: boss + path: /boss + port: 443 + failurePolicy: Ignore + matchPolicy: Exact + name: ingress-hook.admission.test + namespaceSelector: + matchExpressions: + - key: control-plane + operator: DoesNotExist + objectSelector: {} + reinvocationPolicy: Never + rules: + - apiGroups: + - extensions + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + resources: + - ingresses + scope: '*' + sideEffects: Unknown + timeoutSeconds: 30 +kind: List +metadata: + resourceVersion: "" + selfLink: "" diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index c17a970b5..ae8a59cb0 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -6,513 +6,513 @@ class SerialDeployTest < Krane::IntegrationTest ## GLOBAL CONTEXT MANIPULATION TESTS # This can be run in parallel if we allow passing the config file path to DeployTask.new # See https://github.com/Shopify/krane/pull/428#pullrequestreview-209720675 - def test_unreachable_context - old_config = ENV['KUBECONFIG'] - begin - ENV['KUBECONFIG'] = File.join(__dir__, '../fixtures/kube-config/dummy_config.yml') - kubectl_instance = build_kubectl(timeout: '0.1s') - result = deploy_fixtures('hello-cloud', kubectl_instance: kubectl_instance) - assert_deploy_failure(result) - assert_logs_match_all([ - 'Result: FAILURE', - "Something went wrong connecting to #{TEST_CONTEXT}", - ], in_order: true) - ensure - ENV['KUBECONFIG'] = old_config - end - end - - def test_multiple_configuration_files - old_config = ENV['KUBECONFIG'] - config_file = File.join(__dir__, '../fixtures/kube-config/unknown_config.yml') - ENV['KUBECONFIG'] = config_file - result = deploy_fixtures('hello-cloud') - assert_deploy_failure(result) - assert_logs_match_all([ - 'Result: FAILURE', - 'Configuration invalid', - "Kubeconfig not found at #{config_file}", - ], in_order: true) - reset_logger - - ENV['KUBECONFIG'] = " : " - result = deploy_fixtures('hello-cloud') - assert_deploy_failure(result) - assert_logs_match_all([ - 'Result: FAILURE', - 'Configuration invalid', - "Kubeconfig file name(s) not set in $KUBECONFIG", - ], in_order: true) - reset_logger - - default_config = "#{Dir.home}/.kube/config" - extra_config = File.join(__dir__, '../fixtures/kube-config/dummy_config.yml') - ENV['KUBECONFIG'] = "#{default_config}:#{extra_config}" - result = deploy_fixtures('hello-cloud', subset: ["configmap-data.yml"]) - assert_deploy_success(result) - ensure - ENV['KUBECONFIG'] = old_config - end - - # We want to be sure that failures to apply resources with potentially sensitive output don't leak any content. - # Currently our only sensitive resource is `Secret`, but we cannot reproduce a failure scenario where the kubectl - # output contains the filename (which would trigger some extra logging). This test stubs `Deployment` to be sensitive - # to recreate such a condition - def test_apply_failure_with_sensitive_resources_hides_template_content - logger.level = 0 - Krane::Deployment.any_instance.expects(:sensitive_template_content?).returns(true).at_least_once - result = deploy_fixtures("hello-cloud", subset: ["web.yml.erb"], render_erb: true) do |fixtures| - bad_port_name = "http_test_is_really_long_and_invalid_chars" - svc = fixtures["web.yml.erb"]["Service"].first - svc["spec"]["ports"].first["targetPort"] = bad_port_name - deployment = fixtures["web.yml.erb"]["Deployment"].first - deployment["spec"]["template"]["spec"]["containers"].first["ports"].first["name"] = bad_port_name - end - assert_deploy_failure(result) - refute_logs_match(%r{Kubectl err:.*something/invalid}) - - assert_logs_match_all([ - "Command failed: apply -f", - /Invalid template: Deployment-web.*\.yml/, - ]) - - refute_logs_match("kind: Deployment") # content of the sensitive template - end - - ## METRICS TESTS - # Metrics tests must be run serially to ensure our global client isn't capturing metrics from other tests - def test_stage_related_metrics_include_custom_tags_from_namespace - hello_cloud = FixtureSetAssertions::HelloCloud.new(@namespace) - kubeclient.patch_namespace(hello_cloud.namespace, metadata: { labels: { foo: 'bar' } }) - metrics = capture_statsd_calls(client: Krane::StatsD.client) do - assert_deploy_success(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"], wait: false)) - end - - %w( - Krane.validate_configuration.duration - Krane.discover_resources.duration - Krane.discover_resources.count - Krane.validate_resources.duration - Krane.initial_status.duration - Krane.priority_resources.duration - Krane.priority_resources.count - Krane.apply_all.duration - Krane.normal_resources.duration - Krane.all_resources.duration - - ).each do |expected_metric| - metric = metrics.find { |m| m.name == expected_metric } - refute_nil(metric, "Metric #{expected_metric} not emitted") - assert_includes(metric.tags, "foo:bar", "Metric #{expected_metric} did not have custom tags") - end - end - - def test_all_expected_statsd_metrics_emitted_with_essential_tags - metrics = capture_statsd_calls(client: Krane::StatsD.client) do - result = deploy_fixtures('hello-cloud', subset: ['configmap-data.yml'], wait: false, sha: 'test-sha') - assert_deploy_success(result) - end - - assert_equal(1, metrics.count { |m| m.type == :_e }, "Expected to find one event metric") - - %w( - Krane.validate_configuration.duration - Krane.discover_resources.duration - Krane.discover_resources.count - Krane.initial_status.duration - Krane.validate_resources.duration - Krane.priority_resources.duration - Krane.priority_resources.count - Krane.apply_all.duration - Krane.normal_resources.duration - Krane.sync.duration - Krane.all_resources.duration - ).each do |expected_metric| - metric = metrics.find { |m| m.name == expected_metric } - refute_nil(metric, "Metric #{expected_metric} not emitted") - assert_includes(metric.tags, "namespace:#{@namespace}", "#{metric.name} is missing namespace tag") - assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}", "#{metric.name} is missing context tag") - assert_includes(metric.tags, "sha:test-sha", "#{metric.name} is missing sha tag") - end - end - - def test_global_deploy_emits_expected_statsd_metrics - metrics = capture_statsd_calls(client: Krane::StatsD.client) do - assert_deploy_success(deploy_global_fixtures('globals')) - end - - assert_equal(1, metrics.count { |m| m.type == :_e }, "Expected to find one event metric") - - %w( - Krane.validate_configuration.duration - Krane.discover_resources.duration - Krane.discover_resources.count - Krane.initial_status.duration - Krane.validate_resources.duration - Krane.apply_all.duration - Krane.normal_resources.duration - Krane.sync.duration - Krane.all_resources.duration - ).each do |expected_metric| - metric = metrics.find { |m| m.name == expected_metric } - refute_nil(metric, "Metric #{expected_metric} not emitted") - assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}", "#{metric.name} is missing context tag") - end - end - - ## BLACK BOX TESTS - # test_global_deploy_black_box_failure is in test/integration/krane_test.rb - # because it does not modify global state. The following two tests modify - # global state and must be run in serially - def test_global_deploy_black_box_success - setup_template_dir("globals") do |target_dir| - flags = "-f #{target_dir} --selector app=krane" - out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") - assert_empty(out) - assert_match("Success", err) - assert_predicate(status, :success?) - end - ensure - build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false) - end - - def test_global_deploy_black_box_timeout - setup_template_dir("globals") do |target_dir| - flags = "-f #{target_dir} --selector app=krane --global-timeout=0.1s" - out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") - assert_empty(out) - assert_match("TIMED OUT", err) - refute_predicate(status, :success?) - assert_equal(status.exitstatus, 70) - end - ensure - build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false) - end - - def test_global_deploy_prune_black_box_success - namespace_name = "test-app" - setup_template_dir("globals") do |target_dir| - flags = "-f #{target_dir} --selector app=krane" - namespace_str = "apiVersion: v1\nkind: Namespace\nmetadata:\n name: #{namespace_name}"\ - "\n labels:\n app: krane" - File.write(File.join(target_dir, "namespace.yml"), namespace_str) - out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") - assert_empty(out) - assert_match("Successfully deployed 3 resource", err) - assert_match(/#{namespace_name}\W+Exists/, err) - assert_match("Success", err) - assert_predicate(status, :success?) - - flags = "-f #{target_dir}/storage_classes.yml --selector app=krane" - out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") - assert_empty(out) - refute_match(namespace_name, err) # Asserting that the namespace is not pruned - assert_match("Pruned 1 resource and successfully deployed 1 resource", err) - assert_predicate(status, :success?) - end - ensure - build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false) - build_kubectl.run("delete", "namespace", namespace_name, use_namespace: false, log_failure: false) - end - - ## TESTS THAT DEPLOY CRDS - # Tests that create CRDs cannot be run in parallel with tests that deploy namespaced resources - # This is because the CRD kind may torn down in the middle of the namespaced deploy, causing it to be seen - # when we build the pruning whitelist, but gone by the time we attempt to list instances for pruning purposes. - # When this happens, the namespaced deploy will fail with an `apply` error. - def test_cr_merging - assert_deploy_success(deploy_global_fixtures("crd", subset: %(mail.yml))) - result = deploy_fixtures("crd", subset: %w(mail_cr.yml)) do |f| - cr = f.dig("mail_cr.yml", "Mail").first - cr["kind"] = add_unique_prefix_for_test(cr["kind"]) - end - assert_deploy_success(result) - - result = deploy_fixtures("crd", subset: %w(mail_cr.yml)) do |f| - cr = f.dig("mail_cr.yml", "Mail").first - cr["spec"]["something"] = 5 - cr["kind"] = add_unique_prefix_for_test(cr["kind"]) - end - assert_deploy_success(result) - end - - def test_custom_resources_predeployed - assert_deploy_success(deploy_global_fixtures("crd", subset: %w(mail.yml things.yml widgets.yml)) do |f| - mail = f.dig("mail.yml", "CustomResourceDefinition").first - mail["metadata"]["annotations"] = {} - - things = f.dig("things.yml", "CustomResourceDefinition").first - things["metadata"]["annotations"] = { - "krane.shopify.io/predeployed" => "true", - } - - widgets = f.dig("widgets.yml", "CustomResourceDefinition").first - widgets["metadata"]["annotations"] = { - "krane.shopify.io/predeployed" => "false", - } - end) - reset_logger - - result = deploy_fixtures("crd", subset: %w(mail_cr.yml things_cr.yml widgets_cr.yml)) do |f| - f.each do |_filename, contents| - contents.each do |_kind, crs| # all of the resources are CRs, so change all of them - crs.each { |cr| cr["kind"] = add_unique_prefix_for_test(cr["kind"]) } - end - end - end - assert_deploy_success(result) - - mail_cr_id = "#{add_unique_prefix_for_test('Mail')}/my-first-mail" - thing_cr_id = "#{add_unique_prefix_for_test('Thing')}/my-first-thing" - widget_cr_id = "#{add_unique_prefix_for_test('Widget')}/my-first-widget" - assert_logs_match_all([ - /Phase 3: Predeploying priority resources/, - /Successfully deployed in \d.\ds: #{mail_cr_id}/, - /Successfully deployed in \d.\ds: #{thing_cr_id}/, - /Phase 4: Deploying all resources/, - /Successfully deployed in \d.\ds: #{mail_cr_id}, #{thing_cr_id}, #{widget_cr_id}/, - ], in_order: true) - refute_logs_match( - /Successfully deployed in \d.\ds: #{widget_cr_id}/, - ) - end - - def test_cr_deploys_without_rollout_conditions_when_none_present - assert_deploy_success(deploy_global_fixtures("crd", subset: %(widgets.yml))) - result = deploy_fixtures("crd", subset: %w(widgets_cr.yml)) do |f| - f.each do |_filename, contents| # all of the resources are CRs, so change all of them - contents.each do |_kind, crs| - crs.each { |cr| cr["kind"] = add_unique_prefix_for_test(cr["kind"]) } - end - end - end - - assert_deploy_success(result) - prefixed_kind = add_unique_prefix_for_test("Widget") - assert_logs_match_all([ - "Don't know how to monitor resources of type #{prefixed_kind}.", - "Assuming #{prefixed_kind}/my-first-widget deployed successfully.", - %r{Widget/my-first-widget\s+Exists}, - ]) - end - - def test_cr_success_with_default_rollout_conditions - assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_default_conditions.yml))) - success_conditions = { - "status" => { - "observedGeneration" => 1, - "conditions" => [ - { - "type" => "Ready", - "reason" => "test", - "message" => "test", - "status" => "True", - }, - ], - }, - } - - result = deploy_fixtures("crd", subset: ["with_default_conditions_cr.yml"]) do |resource| - cr = resource["with_default_conditions_cr.yml"]["Parameterized"].first - cr.merge!(success_conditions) - cr["kind"] = add_unique_prefix_for_test(cr["kind"]) - end - assert_deploy_success(result) - assert_logs_match_all([ - %r{Successfully deployed in .*: #{add_unique_prefix_for_test("Parameterized")}\/with-default-params}, - %r{Parameterized/with-default-params\s+Healthy}, - ]) - end - - def test_cr_success_with_service - filepath = "#{fixture_path('crd')}/service_cr.yml" - out, err, st = build_kubectl.run("create", "-f", filepath, log_failure: true, use_namespace: false) - assert(st.success?, "Failed to create CRD: #{out}\n#{err}") - - assert_deploy_success(deploy_fixtures("crd", subset: %w(web.yml))) - - refute_logs_match(/Predeploying priority resources/) - assert_logs_match_all([/Phase 3: Deploying all resources/]) - ensure - build_kubectl.run("delete", "-f", filepath, use_namespace: false, log_failure: false) - end - - def test_cr_failure_with_default_rollout_conditions - assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_default_conditions.yml))) - failure_conditions = { - "status" => { - "observedGeneration" => 1, - "conditions" => [ - { - "type" => "Failed", - "reason" => "test", - "message" => "custom resource rollout failed", - "status" => "True", - }, - ], - }, - } - - result = deploy_fixtures("crd", subset: ["with_default_conditions_cr.yml"]) do |resource| - cr = resource["with_default_conditions_cr.yml"]["Parameterized"].first - cr.merge!(failure_conditions) - cr["kind"] = add_unique_prefix_for_test(cr["kind"]) - end - assert_deploy_failure(result) - - assert_logs_match_all([ - "Parameterized/with-default-params: FAILED", - "custom resource rollout failed", - "Final status: Unhealthy", - ], in_order: true) - end - - def test_cr_success_with_arbitrary_rollout_conditions - assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_custom_conditions.yml))) - - success_conditions = { - "spec" => {}, - "status" => { - "observedGeneration" => 1, - "test_field" => "success_value", - "condition" => "success_value", - }, - } - - result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml"]) do |resource| - cr = resource["with_custom_conditions_cr.yml"]["Customized"].first - cr["kind"] = add_unique_prefix_for_test(cr["kind"]) - cr.merge!(success_conditions) - end - assert_deploy_success(result) - assert_logs_match_all([ - %r{Successfully deployed in .*: #{add_unique_prefix_for_test("Customized")}\/with-customized-params}, - ]) - end - - def test_cr_failure_with_arbitrary_rollout_conditions - assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_custom_conditions.yml))) - cr = load_fixtures("crd", ["with_custom_conditions_cr.yml"]) - failure_conditions = { - "spec" => {}, - "status" => { - "test_field" => "failure_value", - "error_msg" => "test error message jsonpath", - "observedGeneration" => 1, - "condition" => "failure_value", - }, - } - - result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml"]) do |resource| - cr = resource["with_custom_conditions_cr.yml"]["Customized"].first - cr["kind"] = add_unique_prefix_for_test(cr["kind"]) - cr.merge!(failure_conditions) - end - assert_deploy_failure(result) - assert_logs_match_all([ - "test error message jsonpath", - "test custom error message", - ]) - end - - def test_deploying_crs_with_invalid_crd_conditions_fails - # Since CRDs are not always deployed along with their CRs and krane is not the only way CRDs are - # deployed, we need to model the case where poorly configured rollout_conditions are present before deploying a CR - fixtures = load_fixtures("crd", "with_custom_conditions.yml") - crd = fixtures["with_custom_conditions.yml"]["CustomResourceDefinition"].first - crd["metadata"]["annotations"].merge!(rollout_conditions_annotation_key => "blah") - apply_scope_to_resources(fixtures, labels: "app=krane,test=#{@namespace}") - Tempfile.open([@namespace, ".yml"]) do |f| - f.write(YAML.dump(crd)) - f.fsync - @deployed_global_fixture_paths << f.path - out, err, st = build_kubectl.run("create", "-f", f.path, log_failure: true, use_namespace: false) - assert(st.success?, "Failed to create invalid CRD: #{out}\n#{err}") - end - - result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml", "with_custom_conditions_cr2.yml"]) do |f| - f.each do |_filename, contents| - contents.each do |_kind, crs| # all of the resources are CRs, so change all of them - crs.each { |cr| cr["kind"] = add_unique_prefix_for_test(cr["kind"]) } - end - end - end - assert_deploy_failure(result) - prefixed_name = add_unique_prefix_for_test("Customized-with-customized-params") - assert_logs_match_all([ - /Invalid template: #{prefixed_name}/, - /Rollout conditions are not valid JSON/, - /Invalid template: #{prefixed_name}/, - /Rollout conditions are not valid JSON/, - ], in_order: true) - end - - def test_crd_can_fail - result = deploy_global_fixtures("crd", subset: %(mail.yml)) do |f| - crd = f.dig("mail.yml", "CustomResourceDefinition").first - names = crd.dig("spec", "names") - names["listKind"] = 'Conflict' - end - assert_deploy_success(result) - - second_name = add_unique_prefix_for_test("others") - result = deploy_global_fixtures("crd", subset: %(mail.yml), prune: false) do |f| - crd = f.dig("mail.yml", "CustomResourceDefinition").first - names = crd.dig("spec", "names") - names["listKind"] = "Conflict" - names["plural"] = second_name - crd["metadata"]["name"] = "#{second_name}.stable.example.io" - end - assert_deploy_failure(result) - assert_logs_match_all([ - "Deploying CustomResourceDefinition/#{second_name}.stable.example.io (timeout: 120s)", - "CustomResourceDefinition/#{second_name}.stable.example.io: FAILED", - 'Final status: ListKindConflict ("Conflict" is already in use)', - ]) - end - - def test_global_deploy_validation_catches_namespaced_cr - assert_deploy_success(deploy_global_fixtures("crd", subset: %(mail.yml))) - reset_logger - result = deploy_global_fixtures("crd", subset: %(mail_cr.yml)) do |fixtures| - mail = fixtures["mail_cr.yml"]["Mail"].first - mail["kind"] = add_unique_prefix_for_test(mail["kind"]) - end - assert_deploy_failure(result) - assert_logs_match_all([ - "Phase 1: Initializing deploy", - "Using resource selector app=krane", - "All required parameters and files are present", - "Discovering resources:", - "- #{add_unique_prefix_for_test('Mail')}/#{add_unique_prefix_for_test('my-first-mail')}", - "Result: FAILURE", - "This command cannot deploy namespaced resources", - "Namespaced resources:", - "#{add_unique_prefix_for_test('my-first-mail')} (#{add_unique_prefix_for_test('Mail')})", - ]) - end - - def test_resource_discovery_stops_deploys_when_fetch_resources_kubectl_errs - failure_msg = "Stubbed failure reason" - Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_resources).raises(Krane::FatalKubeAPIError, failure_msg) - assert_deploy_failure(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"])) - - assert_logs_match_all([ - "Result: FAILURE", - failure_msg, - ], in_order: true) - end - - def test_resource_discovery_stops_deploys_when_fetch_crds_kubectl_errs - failure_msg = "Stubbed failure reason" - Krane::ClusterResourceDiscovery.any_instance.expects(:crds).raises(Krane::FatalKubeAPIError, failure_msg) - assert_deploy_failure(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"])) - - assert_logs_match_all([ - "Result: FAILURE", - failure_msg, - ], in_order: true) - end + # def test_unreachable_context + # old_config = ENV['KUBECONFIG'] + # begin + # ENV['KUBECONFIG'] = File.join(__dir__, '../fixtures/kube-config/dummy_config.yml') + # kubectl_instance = build_kubectl(timeout: '0.1s') + # result = deploy_fixtures('hello-cloud', kubectl_instance: kubectl_instance) + # assert_deploy_failure(result) + # assert_logs_match_all([ + # 'Result: FAILURE', + # "Something went wrong connecting to #{TEST_CONTEXT}", + # ], in_order: true) + # ensure + # ENV['KUBECONFIG'] = old_config + # end + # end + + # def test_multiple_configuration_files + # old_config = ENV['KUBECONFIG'] + # config_file = File.join(__dir__, '../fixtures/kube-config/unknown_config.yml') + # ENV['KUBECONFIG'] = config_file + # result = deploy_fixtures('hello-cloud') + # assert_deploy_failure(result) + # assert_logs_match_all([ + # 'Result: FAILURE', + # 'Configuration invalid', + # "Kubeconfig not found at #{config_file}", + # ], in_order: true) + # reset_logger + + # ENV['KUBECONFIG'] = " : " + # result = deploy_fixtures('hello-cloud') + # assert_deploy_failure(result) + # assert_logs_match_all([ + # 'Result: FAILURE', + # 'Configuration invalid', + # "Kubeconfig file name(s) not set in $KUBECONFIG", + # ], in_order: true) + # reset_logger + + # default_config = "#{Dir.home}/.kube/config" + # extra_config = File.join(__dir__, '../fixtures/kube-config/dummy_config.yml') + # ENV['KUBECONFIG'] = "#{default_config}:#{extra_config}" + # result = deploy_fixtures('hello-cloud', subset: ["configmap-data.yml"]) + # assert_deploy_success(result) + # ensure + # ENV['KUBECONFIG'] = old_config + # end + + # # We want to be sure that failures to apply resources with potentially sensitive output don't leak any content. + # # Currently our only sensitive resource is `Secret`, but we cannot reproduce a failure scenario where the kubectl + # # output contains the filename (which would trigger some extra logging). This test stubs `Deployment` to be sensitive + # # to recreate such a condition + # def test_apply_failure_with_sensitive_resources_hides_template_content + # logger.level = 0 + # Krane::Deployment.any_instance.expects(:sensitive_template_content?).returns(true).at_least_once + # result = deploy_fixtures("hello-cloud", subset: ["web.yml.erb"], render_erb: true) do |fixtures| + # bad_port_name = "http_test_is_really_long_and_invalid_chars" + # svc = fixtures["web.yml.erb"]["Service"].first + # svc["spec"]["ports"].first["targetPort"] = bad_port_name + # deployment = fixtures["web.yml.erb"]["Deployment"].first + # deployment["spec"]["template"]["spec"]["containers"].first["ports"].first["name"] = bad_port_name + # end + # assert_deploy_failure(result) + # refute_logs_match(%r{Kubectl err:.*something/invalid}) + + # assert_logs_match_all([ + # "Command failed: apply -f", + # /Invalid template: Deployment-web.*\.yml/, + # ]) + + # refute_logs_match("kind: Deployment") # content of the sensitive template + # end + + # ## METRICS TESTS + # # Metrics tests must be run serially to ensure our global client isn't capturing metrics from other tests + # def test_stage_related_metrics_include_custom_tags_from_namespace + # hello_cloud = FixtureSetAssertions::HelloCloud.new(@namespace) + # kubeclient.patch_namespace(hello_cloud.namespace, metadata: { labels: { foo: 'bar' } }) + # metrics = capture_statsd_calls(client: Krane::StatsD.client) do + # assert_deploy_success(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"], wait: false)) + # end + + # %w( + # Krane.validate_configuration.duration + # Krane.discover_resources.duration + # Krane.discover_resources.count + # Krane.validate_resources.duration + # Krane.initial_status.duration + # Krane.priority_resources.duration + # Krane.priority_resources.count + # Krane.apply_all.duration + # Krane.normal_resources.duration + # Krane.all_resources.duration + + # ).each do |expected_metric| + # metric = metrics.find { |m| m.name == expected_metric } + # refute_nil(metric, "Metric #{expected_metric} not emitted") + # assert_includes(metric.tags, "foo:bar", "Metric #{expected_metric} did not have custom tags") + # end + # end + + # def test_all_expected_statsd_metrics_emitted_with_essential_tags + # metrics = capture_statsd_calls(client: Krane::StatsD.client) do + # result = deploy_fixtures('hello-cloud', subset: ['configmap-data.yml'], wait: false, sha: 'test-sha') + # assert_deploy_success(result) + # end + + # assert_equal(1, metrics.count { |m| m.type == :_e }, "Expected to find one event metric") + + # %w( + # Krane.validate_configuration.duration + # Krane.discover_resources.duration + # Krane.discover_resources.count + # Krane.initial_status.duration + # Krane.validate_resources.duration + # Krane.priority_resources.duration + # Krane.priority_resources.count + # Krane.apply_all.duration + # Krane.normal_resources.duration + # Krane.sync.duration + # Krane.all_resources.duration + # ).each do |expected_metric| + # metric = metrics.find { |m| m.name == expected_metric } + # refute_nil(metric, "Metric #{expected_metric} not emitted") + # assert_includes(metric.tags, "namespace:#{@namespace}", "#{metric.name} is missing namespace tag") + # assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}", "#{metric.name} is missing context tag") + # assert_includes(metric.tags, "sha:test-sha", "#{metric.name} is missing sha tag") + # end + # end + + # def test_global_deploy_emits_expected_statsd_metrics + # metrics = capture_statsd_calls(client: Krane::StatsD.client) do + # assert_deploy_success(deploy_global_fixtures('globals')) + # end + + # assert_equal(1, metrics.count { |m| m.type == :_e }, "Expected to find one event metric") + + # %w( + # Krane.validate_configuration.duration + # Krane.discover_resources.duration + # Krane.discover_resources.count + # Krane.initial_status.duration + # Krane.validate_resources.duration + # Krane.apply_all.duration + # Krane.normal_resources.duration + # Krane.sync.duration + # Krane.all_resources.duration + # ).each do |expected_metric| + # metric = metrics.find { |m| m.name == expected_metric } + # refute_nil(metric, "Metric #{expected_metric} not emitted") + # assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}", "#{metric.name} is missing context tag") + # end + # end + + # ## BLACK BOX TESTS + # # test_global_deploy_black_box_failure is in test/integration/krane_test.rb + # # because it does not modify global state. The following two tests modify + # # global state and must be run in serially + # def test_global_deploy_black_box_success + # setup_template_dir("globals") do |target_dir| + # flags = "-f #{target_dir} --selector app=krane" + # out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") + # assert_empty(out) + # assert_match("Success", err) + # assert_predicate(status, :success?) + # end + # ensure + # build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false) + # end + + # def test_global_deploy_black_box_timeout + # setup_template_dir("globals") do |target_dir| + # flags = "-f #{target_dir} --selector app=krane --global-timeout=0.1s" + # out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") + # assert_empty(out) + # assert_match("TIMED OUT", err) + # refute_predicate(status, :success?) + # assert_equal(status.exitstatus, 70) + # end + # ensure + # build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false) + # end + + # def test_global_deploy_prune_black_box_success + # namespace_name = "test-app" + # setup_template_dir("globals") do |target_dir| + # flags = "-f #{target_dir} --selector app=krane" + # namespace_str = "apiVersion: v1\nkind: Namespace\nmetadata:\n name: #{namespace_name}"\ + # "\n labels:\n app: krane" + # File.write(File.join(target_dir, "namespace.yml"), namespace_str) + # out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") + # assert_empty(out) + # assert_match("Successfully deployed 3 resource", err) + # assert_match(/#{namespace_name}\W+Exists/, err) + # assert_match("Success", err) + # assert_predicate(status, :success?) + + # flags = "-f #{target_dir}/storage_classes.yml --selector app=krane" + # out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") + # assert_empty(out) + # refute_match(namespace_name, err) # Asserting that the namespace is not pruned + # assert_match("Pruned 1 resource and successfully deployed 1 resource", err) + # assert_predicate(status, :success?) + # end + # ensure + # build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false) + # build_kubectl.run("delete", "namespace", namespace_name, use_namespace: false, log_failure: false) + # end + + # ## TESTS THAT DEPLOY CRDS + # # Tests that create CRDs cannot be run in parallel with tests that deploy namespaced resources + # # This is because the CRD kind may torn down in the middle of the namespaced deploy, causing it to be seen + # # when we build the pruning whitelist, but gone by the time we attempt to list instances for pruning purposes. + # # When this happens, the namespaced deploy will fail with an `apply` error. + # def test_cr_merging + # assert_deploy_success(deploy_global_fixtures("crd", subset: %(mail.yml))) + # result = deploy_fixtures("crd", subset: %w(mail_cr.yml)) do |f| + # cr = f.dig("mail_cr.yml", "Mail").first + # cr["kind"] = add_unique_prefix_for_test(cr["kind"]) + # end + # assert_deploy_success(result) + + # result = deploy_fixtures("crd", subset: %w(mail_cr.yml)) do |f| + # cr = f.dig("mail_cr.yml", "Mail").first + # cr["spec"]["something"] = 5 + # cr["kind"] = add_unique_prefix_for_test(cr["kind"]) + # end + # assert_deploy_success(result) + # end + + # def test_custom_resources_predeployed + # assert_deploy_success(deploy_global_fixtures("crd", subset: %w(mail.yml things.yml widgets.yml)) do |f| + # mail = f.dig("mail.yml", "CustomResourceDefinition").first + # mail["metadata"]["annotations"] = {} + + # things = f.dig("things.yml", "CustomResourceDefinition").first + # things["metadata"]["annotations"] = { + # "krane.shopify.io/predeployed" => "true", + # } + + # widgets = f.dig("widgets.yml", "CustomResourceDefinition").first + # widgets["metadata"]["annotations"] = { + # "krane.shopify.io/predeployed" => "false", + # } + # end) + # reset_logger + + # result = deploy_fixtures("crd", subset: %w(mail_cr.yml things_cr.yml widgets_cr.yml)) do |f| + # f.each do |_filename, contents| + # contents.each do |_kind, crs| # all of the resources are CRs, so change all of them + # crs.each { |cr| cr["kind"] = add_unique_prefix_for_test(cr["kind"]) } + # end + # end + # end + # assert_deploy_success(result) + + # mail_cr_id = "#{add_unique_prefix_for_test('Mail')}/my-first-mail" + # thing_cr_id = "#{add_unique_prefix_for_test('Thing')}/my-first-thing" + # widget_cr_id = "#{add_unique_prefix_for_test('Widget')}/my-first-widget" + # assert_logs_match_all([ + # /Phase 3: Predeploying priority resources/, + # /Successfully deployed in \d.\ds: #{mail_cr_id}/, + # /Successfully deployed in \d.\ds: #{thing_cr_id}/, + # /Phase 4: Deploying all resources/, + # /Successfully deployed in \d.\ds: #{mail_cr_id}, #{thing_cr_id}, #{widget_cr_id}/, + # ], in_order: true) + # refute_logs_match( + # /Successfully deployed in \d.\ds: #{widget_cr_id}/, + # ) + # end + + # def test_cr_deploys_without_rollout_conditions_when_none_present + # assert_deploy_success(deploy_global_fixtures("crd", subset: %(widgets.yml))) + # result = deploy_fixtures("crd", subset: %w(widgets_cr.yml)) do |f| + # f.each do |_filename, contents| # all of the resources are CRs, so change all of them + # contents.each do |_kind, crs| + # crs.each { |cr| cr["kind"] = add_unique_prefix_for_test(cr["kind"]) } + # end + # end + # end + + # assert_deploy_success(result) + # prefixed_kind = add_unique_prefix_for_test("Widget") + # assert_logs_match_all([ + # "Don't know how to monitor resources of type #{prefixed_kind}.", + # "Assuming #{prefixed_kind}/my-first-widget deployed successfully.", + # %r{Widget/my-first-widget\s+Exists}, + # ]) + # end + + # def test_cr_success_with_default_rollout_conditions + # assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_default_conditions.yml))) + # success_conditions = { + # "status" => { + # "observedGeneration" => 1, + # "conditions" => [ + # { + # "type" => "Ready", + # "reason" => "test", + # "message" => "test", + # "status" => "True", + # }, + # ], + # }, + # } + + # result = deploy_fixtures("crd", subset: ["with_default_conditions_cr.yml"]) do |resource| + # cr = resource["with_default_conditions_cr.yml"]["Parameterized"].first + # cr.merge!(success_conditions) + # cr["kind"] = add_unique_prefix_for_test(cr["kind"]) + # end + # assert_deploy_success(result) + # assert_logs_match_all([ + # %r{Successfully deployed in .*: #{add_unique_prefix_for_test("Parameterized")}\/with-default-params}, + # %r{Parameterized/with-default-params\s+Healthy}, + # ]) + # end + + # def test_cr_success_with_service + # filepath = "#{fixture_path('crd')}/service_cr.yml" + # out, err, st = build_kubectl.run("create", "-f", filepath, log_failure: true, use_namespace: false) + # assert(st.success?, "Failed to create CRD: #{out}\n#{err}") + + # assert_deploy_success(deploy_fixtures("crd", subset: %w(web.yml))) + + # refute_logs_match(/Predeploying priority resources/) + # assert_logs_match_all([/Phase 3: Deploying all resources/]) + # ensure + # build_kubectl.run("delete", "-f", filepath, use_namespace: false, log_failure: false) + # end + + # def test_cr_failure_with_default_rollout_conditions + # assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_default_conditions.yml))) + # failure_conditions = { + # "status" => { + # "observedGeneration" => 1, + # "conditions" => [ + # { + # "type" => "Failed", + # "reason" => "test", + # "message" => "custom resource rollout failed", + # "status" => "True", + # }, + # ], + # }, + # } + + # result = deploy_fixtures("crd", subset: ["with_default_conditions_cr.yml"]) do |resource| + # cr = resource["with_default_conditions_cr.yml"]["Parameterized"].first + # cr.merge!(failure_conditions) + # cr["kind"] = add_unique_prefix_for_test(cr["kind"]) + # end + # assert_deploy_failure(result) + + # assert_logs_match_all([ + # "Parameterized/with-default-params: FAILED", + # "custom resource rollout failed", + # "Final status: Unhealthy", + # ], in_order: true) + # end + + # def test_cr_success_with_arbitrary_rollout_conditions + # assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_custom_conditions.yml))) + + # success_conditions = { + # "spec" => {}, + # "status" => { + # "observedGeneration" => 1, + # "test_field" => "success_value", + # "condition" => "success_value", + # }, + # } + + # result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml"]) do |resource| + # cr = resource["with_custom_conditions_cr.yml"]["Customized"].first + # cr["kind"] = add_unique_prefix_for_test(cr["kind"]) + # cr.merge!(success_conditions) + # end + # assert_deploy_success(result) + # assert_logs_match_all([ + # %r{Successfully deployed in .*: #{add_unique_prefix_for_test("Customized")}\/with-customized-params}, + # ]) + # end + + # def test_cr_failure_with_arbitrary_rollout_conditions + # assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_custom_conditions.yml))) + # cr = load_fixtures("crd", ["with_custom_conditions_cr.yml"]) + # failure_conditions = { + # "spec" => {}, + # "status" => { + # "test_field" => "failure_value", + # "error_msg" => "test error message jsonpath", + # "observedGeneration" => 1, + # "condition" => "failure_value", + # }, + # } + + # result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml"]) do |resource| + # cr = resource["with_custom_conditions_cr.yml"]["Customized"].first + # cr["kind"] = add_unique_prefix_for_test(cr["kind"]) + # cr.merge!(failure_conditions) + # end + # assert_deploy_failure(result) + # assert_logs_match_all([ + # "test error message jsonpath", + # "test custom error message", + # ]) + # end + + # def test_deploying_crs_with_invalid_crd_conditions_fails + # # Since CRDs are not always deployed along with their CRs and krane is not the only way CRDs are + # # deployed, we need to model the case where poorly configured rollout_conditions are present before deploying a CR + # fixtures = load_fixtures("crd", "with_custom_conditions.yml") + # crd = fixtures["with_custom_conditions.yml"]["CustomResourceDefinition"].first + # crd["metadata"]["annotations"].merge!(rollout_conditions_annotation_key => "blah") + # apply_scope_to_resources(fixtures, labels: "app=krane,test=#{@namespace}") + # Tempfile.open([@namespace, ".yml"]) do |f| + # f.write(YAML.dump(crd)) + # f.fsync + # @deployed_global_fixture_paths << f.path + # out, err, st = build_kubectl.run("create", "-f", f.path, log_failure: true, use_namespace: false) + # assert(st.success?, "Failed to create invalid CRD: #{out}\n#{err}") + # end + + # result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml", "with_custom_conditions_cr2.yml"]) do |f| + # f.each do |_filename, contents| + # contents.each do |_kind, crs| # all of the resources are CRs, so change all of them + # crs.each { |cr| cr["kind"] = add_unique_prefix_for_test(cr["kind"]) } + # end + # end + # end + # assert_deploy_failure(result) + # prefixed_name = add_unique_prefix_for_test("Customized-with-customized-params") + # assert_logs_match_all([ + # /Invalid template: #{prefixed_name}/, + # /Rollout conditions are not valid JSON/, + # /Invalid template: #{prefixed_name}/, + # /Rollout conditions are not valid JSON/, + # ], in_order: true) + # end + + # def test_crd_can_fail + # result = deploy_global_fixtures("crd", subset: %(mail.yml)) do |f| + # crd = f.dig("mail.yml", "CustomResourceDefinition").first + # names = crd.dig("spec", "names") + # names["listKind"] = 'Conflict' + # end + # assert_deploy_success(result) + + # second_name = add_unique_prefix_for_test("others") + # result = deploy_global_fixtures("crd", subset: %(mail.yml), prune: false) do |f| + # crd = f.dig("mail.yml", "CustomResourceDefinition").first + # names = crd.dig("spec", "names") + # names["listKind"] = "Conflict" + # names["plural"] = second_name + # crd["metadata"]["name"] = "#{second_name}.stable.example.io" + # end + # assert_deploy_failure(result) + # assert_logs_match_all([ + # "Deploying CustomResourceDefinition/#{second_name}.stable.example.io (timeout: 120s)", + # "CustomResourceDefinition/#{second_name}.stable.example.io: FAILED", + # 'Final status: ListKindConflict ("Conflict" is already in use)', + # ]) + # end + + # def test_global_deploy_validation_catches_namespaced_cr + # assert_deploy_success(deploy_global_fixtures("crd", subset: %(mail.yml))) + # reset_logger + # result = deploy_global_fixtures("crd", subset: %(mail_cr.yml)) do |fixtures| + # mail = fixtures["mail_cr.yml"]["Mail"].first + # mail["kind"] = add_unique_prefix_for_test(mail["kind"]) + # end + # assert_deploy_failure(result) + # assert_logs_match_all([ + # "Phase 1: Initializing deploy", + # "Using resource selector app=krane", + # "All required parameters and files are present", + # "Discovering resources:", + # "- #{add_unique_prefix_for_test('Mail')}/#{add_unique_prefix_for_test('my-first-mail')}", + # "Result: FAILURE", + # "This command cannot deploy namespaced resources", + # "Namespaced resources:", + # "#{add_unique_prefix_for_test('my-first-mail')} (#{add_unique_prefix_for_test('Mail')})", + # ]) + # end + + # def test_resource_discovery_stops_deploys_when_fetch_resources_kubectl_errs + # failure_msg = "Stubbed failure reason" + # Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_resources).raises(Krane::FatalKubeAPIError, failure_msg) + # assert_deploy_failure(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"])) + + # assert_logs_match_all([ + # "Result: FAILURE", + # failure_msg, + # ], in_order: true) + # end + + # def test_resource_discovery_stops_deploys_when_fetch_crds_kubectl_errs + # failure_msg = "Stubbed failure reason" + # Krane::ClusterResourceDiscovery.any_instance.expects(:crds).raises(Krane::FatalKubeAPIError, failure_msg) + # assert_deploy_failure(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"])) + + # assert_logs_match_all([ + # "Result: FAILURE", + # failure_msg, + # ], in_order: true) + # end def test_batch_dry_run_apply_failure_falls_back_to_individual_resource_dry_run_validation Krane::KubernetesResource.any_instance.expects(:validate_definition).with do |kwargs| @@ -531,6 +531,13 @@ def test_batch_dry_run_apply_success_precludes_individual_resource_dry_run_valid deploy_fixtures("hello-cloud", subset: %w(secret.yml)) end + # def test_resources_with_side_effect_inducing_webhooks_are_not_server_side_dry_run + # Krane::Kubectl.any_instance.expects(:run).with("get", "mutatingwebhookconfigurations", "-ojson", + # use_namespace: false, raise_if_not_found: true).returns({}) + + # end + # end + private def rollout_conditions_annotation_key diff --git a/test/integration-serial/serial_task_run_test.rb b/test/integration-serial/serial_task_run_test.rb index cb67eba57..c05eda4d5 100644 --- a/test/integration-serial/serial_task_run_test.rb +++ b/test/integration-serial/serial_task_run_test.rb @@ -6,79 +6,79 @@ class SerialTaskRunTest < Krane::IntegrationTest include StatsD::Instrument::Assertions # Mocha is not thread-safe: https://github.com/freerange/mocha#thread-safety - def test_run_without_verify_result_fails_if_pod_was_not_created - deploy_task_template - task_runner = build_task_runner + # def test_run_without_verify_result_fails_if_pod_was_not_created + # deploy_task_template + # task_runner = build_task_runner - # Sketchy, but stubbing the kubeclient doesn't work (and wouldn't be concurrency-friendly) - # Finding a way to reliably trigger a create failure would be much better, if possible - mock = mock() - template = kubeclient.get_pod_template('hello-cloud-template-runner', @namespace) - mock.expects(:get_pod_template).returns(template) - mock.expects(:create_pod).raises(Kubeclient::HttpError.new("409", "Pod with same name exists", {})) - task_runner.instance_variable_set(:@kubeclient, mock) + # # Sketchy, but stubbing the kubeclient doesn't work (and wouldn't be concurrency-friendly) + # # Finding a way to reliably trigger a create failure would be much better, if possible + # mock = mock() + # template = kubeclient.get_pod_template('hello-cloud-template-runner', @namespace) + # mock.expects(:get_pod_template).returns(template) + # mock.expects(:create_pod).raises(Kubeclient::HttpError.new("409", "Pod with same name exists", {})) + # task_runner.instance_variable_set(:@kubeclient, mock) - result = task_runner.run(**run_params(verify_result: false)) - assert_task_run_failure(result) + # result = task_runner.run(**run_params(verify_result: false)) + # assert_task_run_failure(result) - assert_logs_match_all([ - "Running pod", - "Result: FAILURE", - "Failed to create pod", - "Kubeclient::HttpError: Pod with same name exists", - ], in_order: true) - end + # assert_logs_match_all([ + # "Running pod", + # "Result: FAILURE", + # "Failed to create pod", + # "Kubeclient::HttpError: Pod with same name exists", + # ], in_order: true) + # end - # Run statsd tests in serial because capture_statsd_calls modifies global state in a way - # that makes capturing metrics across parallel runs unreliable - def test_failure_statsd_metric_emitted - bad_ns = "missing" - task_runner = build_task_runner(ns: bad_ns) + # # Run statsd tests in serial because capture_statsd_calls modifies global state in a way + # # that makes capturing metrics across parallel runs unreliable + # def test_failure_statsd_metric_emitted + # bad_ns = "missing" + # task_runner = build_task_runner(ns: bad_ns) - metrics = capture_statsd_calls(client: Krane::StatsD.client) do - result = task_runner.run(**run_params) - assert_task_run_failure(result) - end + # metrics = capture_statsd_calls(client: Krane::StatsD.client) do + # result = task_runner.run(**run_params) + # assert_task_run_failure(result) + # end - metric = metrics.find do |m| - m.name == "Krane.task_runner.duration" && m.tags.include?("namespace:#{bad_ns}") - end - assert(metric, "No result metric found for this test") - assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}") - assert_includes(metric.tags, "status:failure") - end + # metric = metrics.find do |m| + # m.name == "Krane.task_runner.duration" && m.tags.include?("namespace:#{bad_ns}") + # end + # assert(metric, "No result metric found for this test") + # assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}") + # assert_includes(metric.tags, "status:failure") + # end - def test_success_statsd_metric_emitted - deploy_task_template - task_runner = build_task_runner + # def test_success_statsd_metric_emitted + # deploy_task_template + # task_runner = build_task_runner - metrics = capture_statsd_calls(client: Krane::StatsD.client) do - result = task_runner.run(**run_params.merge(verify_result: false)) - assert_task_run_success(result) - end + # metrics = capture_statsd_calls(client: Krane::StatsD.client) do + # result = task_runner.run(**run_params.merge(verify_result: false)) + # assert_task_run_success(result) + # end - metric = metrics.find do |m| - m.name == "Krane.task_runner.duration" && m.tags.include?("namespace:#{@namespace}") - end - assert(metric, "No result metric found for this test") - assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}") - assert_includes(metric.tags, "status:success") - end + # metric = metrics.find do |m| + # m.name == "Krane.task_runner.duration" && m.tags.include?("namespace:#{@namespace}") + # end + # assert(metric, "No result metric found for this test") + # assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}") + # assert_includes(metric.tags, "status:success") + # end - def test_timedout_statsd_metric_emitted - deploy_task_template - task_runner = build_task_runner(global_timeout: 0) + # def test_timedout_statsd_metric_emitted + # deploy_task_template + # task_runner = build_task_runner(global_timeout: 0) - metrics = capture_statsd_calls(client: Krane::StatsD.client) do - result = task_runner.run(**run_params.merge(arguments: ["sleep 5"])) - assert_task_run_failure(result, :timed_out) - end + # metrics = capture_statsd_calls(client: Krane::StatsD.client) do + # result = task_runner.run(**run_params.merge(arguments: ["sleep 5"])) + # assert_task_run_failure(result, :timed_out) + # end - metric = metrics.find do |m| - m.name == "Krane.task_runner.duration" && m.tags.include?("namespace:#{@namespace}") - end - assert(metric, "No result metric found for this test") - assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}") - assert_includes(metric.tags, "status:timeout") - end + # metric = metrics.find do |m| + # m.name == "Krane.task_runner.duration" && m.tags.include?("namespace:#{@namespace}") + # end + # assert(metric, "No result metric found for this test") + # assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}") + # assert_includes(metric.tags, "status:timeout") + # end end From f09f26a3fc8f0520ba45d573426a5783d7bd3933 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Tue, 2 Feb 2021 15:04:01 -0500 Subject: [PATCH 02/27] more work --- lib/krane/deploy_task.rb | 2 +- lib/krane/kubernetes_resource/config_map.rb | 4 - .../for_serial_deploy_tests/ingress_hook.yml | 48 + .../mutating_webhooks/ingress_hook.yml | 42 - test/integration-serial/serial_deploy_test.rb | 1027 ++++++++--------- .../serial_task_run_test.rb | 126 +- 6 files changed, 622 insertions(+), 627 deletions(-) create mode 100644 test/fixtures/for_serial_deploy_tests/ingress_hook.yml delete mode 100644 test/fixtures/mutating_webhooks/ingress_hook.yml diff --git a/lib/krane/deploy_task.rb b/lib/krane/deploy_task.rb index 52e117658..44dc4bcdf 100644 --- a/lib/krane/deploy_task.rb +++ b/lib/krane/deploy_task.rb @@ -298,7 +298,7 @@ def partition_dry_run_resources(resources) individuals += resources.select do |r| (r.group == group || group == '*' || match_policy == "Equivalent") && (r.version == version || version == '*' || match_policy == "Equivalent") && - (r.type.downcase == kind.downcase ) + (r.type.downcase == kind.downcase) end resources -= individuals end diff --git a/lib/krane/kubernetes_resource/config_map.rb b/lib/krane/kubernetes_resource/config_map.rb index 364656922..098f48050 100644 --- a/lib/krane/kubernetes_resource/config_map.rb +++ b/lib/krane/kubernetes_resource/config_map.rb @@ -7,10 +7,6 @@ def deploy_succeeded? exists? end - def status - exists? ? "Available" : "Not Found" - end - def deploy_failed? false end diff --git a/test/fixtures/for_serial_deploy_tests/ingress_hook.yml b/test/fixtures/for_serial_deploy_tests/ingress_hook.yml new file mode 100644 index 000000000..4fba2ea5d --- /dev/null +++ b/test/fixtures/for_serial_deploy_tests/ingress_hook.yml @@ -0,0 +1,48 @@ +--- +apiVersion: v1 +items: +- apiVersion: admissionregistration.k8s.io/v1 + kind: MutatingWebhookConfiguration + metadata: + creationTimestamp: '2020-08-04T19:49:44Z' + generation: 2 + name: oauthboss-webhook-configuration + resourceVersion: '3115431' + selfLink: "/apis/admissionregistration.k8s.io/v1/mutatingwebhookconfigurations/oauthboss-webhook-configuration" + uid: 7afce875-5175-430d-95aa-de8ec97f15b0 + webhooks: + - admissionReviewVersions: + - v1beta1 + clientConfig: + caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUMwakNDQWJxZ0F3SUJBZ0lCQURBTkJna3Foa2lHOXcwQkFRc0ZBREFhTVJnd0ZnWURWUVFERXc5M1pXSm8KYjI5ckxXTmxjblF0WTJFd0hoY05NakF3T0RBME1UazBPVFExV2hjTk16QXdPREF5TVRrME9UUTFXakFhTVJndwpGZ1lEVlFRREV3OTNaV0pvYjI5ckxXTmxjblF0WTJFd2dnRWlNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUJEd0F3CmdnRUtBb0lCQVFDOGs1MjNobEFsald4RitITk8wWk9qQTZ4MGNSa1RQVHdkZm9nK3JGOXZ3Y21XRTBFT3kyYlUKZ3BNRU5SQzVpVzRxS1pCYUJ6MEU1bXcvbWNjcGNrbXR1V05pODVML0hzUjVlbWR2aWdUamJYb0RMeWxEdjNMLwpRSlVWOVRIYWFhaXh5cmFMNWhIRUw4bTBhTFNZcWc5MktxODJnWXB0aTB5azBnU1BqdjhUTHBFazhvdWp2L01YCnVJZWtQQzZOTWFDaU1BY1FBTVRyc1NiSmZqL0x0WGpwWk9FM2xCNWpTazNDUW1sT0t5Mm8zaDd6WTZ1RFNtN28KY3orMkhiUXhybCt1SkcrY1B4WVdoWXZReXpLWGViWU11ajd5Wm5FRmI2WFc3dC9ROVo2VmFzMi9DS1Z4Yll4LwppdXEvbU1xZk4xZnNvUitJTmVXVlJ3YlJ0dUJ5VkljRkFnTUJBQUdqSXpBaE1BNEdBMVVkRHdFQi93UUVBd0lDCnBEQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRQXc4VWQwOWRYR09kQkwKYnQ5M3JuYk41VzBkMkJUZk43OXJ6SVpadHlmMjREQVQ1SGc1QVV2Qmc4QjJNaFRqdU50VlN6dzN4bGV3TDk3aQo0Wk9JWGJPL3hZTWp3OXRibldDY2o0MlVKRnJUOEN2eVZEekExeS9VVE1lMFpyU3lOUGpWTlBaMmUrb3IvRFdkCmRtbmg0THZ6c3VNbVBPZk96NmUwZmljMzdtLzZjL3JZV0l6aTJYWWRwSllHa1J1eW51UlVUbDZpUjBOa0xXaGYKc25aNll6TnFXT2YvNi9xdHduNkhVckttYWN3L2Q3blBNSDRpQTFId1MyZFh6WkxVR3NVZkZIREFkU2FVSWR3cApsZDFUUjErQjVyMnhiUW95eVo4YnhuTk5Ud04rU2dFZGc5MjJKUFlaRkZFWldDL1NXOWVYcUZnTWM3RlQyMHh4ClE1NW1qdHRICi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K + service: + name: oauthboss + namespace: cloudbosses + path: "/oauthboss" + port: 443 + failurePolicy: Ignore + matchPolicy: Exact + name: oauthboss.cloudbosses.admission.online.shopify.com + namespaceSelector: + matchExpressions: + - key: control-plane + operator: DoesNotExist + objectSelector: {} + reinvocationPolicy: Never + rules: + - apiGroups: + - extensions + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + resources: + - ingresses + scope: "*" + sideEffects: Unknown + timeoutSeconds: 30 +kind: List +metadata: + resourceVersion: '' + selfLink: '' diff --git a/test/fixtures/mutating_webhooks/ingress_hook.yml b/test/fixtures/mutating_webhooks/ingress_hook.yml deleted file mode 100644 index a629dccba..000000000 --- a/test/fixtures/mutating_webhooks/ingress_hook.yml +++ /dev/null @@ -1,42 +0,0 @@ -apiVersion: v1 -items: -- apiVersion: admissionregistration.k8s.io/v1 - kind: MutatingWebhookConfiguration - metadata: - name: ingress-webhook-configuration - webhooks: - - admissionReviewVersions: - - v1beta1 - clientConfig: - caBundle: BUNDLE - service: - name: boss - namespace: boss - path: /boss - port: 443 - failurePolicy: Ignore - matchPolicy: Exact - name: ingress-hook.admission.test - namespaceSelector: - matchExpressions: - - key: control-plane - operator: DoesNotExist - objectSelector: {} - reinvocationPolicy: Never - rules: - - apiGroups: - - extensions - apiVersions: - - v1beta1 - operations: - - CREATE - - UPDATE - resources: - - ingresses - scope: '*' - sideEffects: Unknown - timeoutSeconds: 30 -kind: List -metadata: - resourceVersion: "" - selfLink: "" diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index ae8a59cb0..2da319a2f 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -6,513 +6,514 @@ class SerialDeployTest < Krane::IntegrationTest ## GLOBAL CONTEXT MANIPULATION TESTS # This can be run in parallel if we allow passing the config file path to DeployTask.new # See https://github.com/Shopify/krane/pull/428#pullrequestreview-209720675 - # def test_unreachable_context - # old_config = ENV['KUBECONFIG'] - # begin - # ENV['KUBECONFIG'] = File.join(__dir__, '../fixtures/kube-config/dummy_config.yml') - # kubectl_instance = build_kubectl(timeout: '0.1s') - # result = deploy_fixtures('hello-cloud', kubectl_instance: kubectl_instance) - # assert_deploy_failure(result) - # assert_logs_match_all([ - # 'Result: FAILURE', - # "Something went wrong connecting to #{TEST_CONTEXT}", - # ], in_order: true) - # ensure - # ENV['KUBECONFIG'] = old_config - # end - # end - - # def test_multiple_configuration_files - # old_config = ENV['KUBECONFIG'] - # config_file = File.join(__dir__, '../fixtures/kube-config/unknown_config.yml') - # ENV['KUBECONFIG'] = config_file - # result = deploy_fixtures('hello-cloud') - # assert_deploy_failure(result) - # assert_logs_match_all([ - # 'Result: FAILURE', - # 'Configuration invalid', - # "Kubeconfig not found at #{config_file}", - # ], in_order: true) - # reset_logger - - # ENV['KUBECONFIG'] = " : " - # result = deploy_fixtures('hello-cloud') - # assert_deploy_failure(result) - # assert_logs_match_all([ - # 'Result: FAILURE', - # 'Configuration invalid', - # "Kubeconfig file name(s) not set in $KUBECONFIG", - # ], in_order: true) - # reset_logger - - # default_config = "#{Dir.home}/.kube/config" - # extra_config = File.join(__dir__, '../fixtures/kube-config/dummy_config.yml') - # ENV['KUBECONFIG'] = "#{default_config}:#{extra_config}" - # result = deploy_fixtures('hello-cloud', subset: ["configmap-data.yml"]) - # assert_deploy_success(result) - # ensure - # ENV['KUBECONFIG'] = old_config - # end - - # # We want to be sure that failures to apply resources with potentially sensitive output don't leak any content. - # # Currently our only sensitive resource is `Secret`, but we cannot reproduce a failure scenario where the kubectl - # # output contains the filename (which would trigger some extra logging). This test stubs `Deployment` to be sensitive - # # to recreate such a condition - # def test_apply_failure_with_sensitive_resources_hides_template_content - # logger.level = 0 - # Krane::Deployment.any_instance.expects(:sensitive_template_content?).returns(true).at_least_once - # result = deploy_fixtures("hello-cloud", subset: ["web.yml.erb"], render_erb: true) do |fixtures| - # bad_port_name = "http_test_is_really_long_and_invalid_chars" - # svc = fixtures["web.yml.erb"]["Service"].first - # svc["spec"]["ports"].first["targetPort"] = bad_port_name - # deployment = fixtures["web.yml.erb"]["Deployment"].first - # deployment["spec"]["template"]["spec"]["containers"].first["ports"].first["name"] = bad_port_name - # end - # assert_deploy_failure(result) - # refute_logs_match(%r{Kubectl err:.*something/invalid}) - - # assert_logs_match_all([ - # "Command failed: apply -f", - # /Invalid template: Deployment-web.*\.yml/, - # ]) - - # refute_logs_match("kind: Deployment") # content of the sensitive template - # end - - # ## METRICS TESTS - # # Metrics tests must be run serially to ensure our global client isn't capturing metrics from other tests - # def test_stage_related_metrics_include_custom_tags_from_namespace - # hello_cloud = FixtureSetAssertions::HelloCloud.new(@namespace) - # kubeclient.patch_namespace(hello_cloud.namespace, metadata: { labels: { foo: 'bar' } }) - # metrics = capture_statsd_calls(client: Krane::StatsD.client) do - # assert_deploy_success(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"], wait: false)) - # end - - # %w( - # Krane.validate_configuration.duration - # Krane.discover_resources.duration - # Krane.discover_resources.count - # Krane.validate_resources.duration - # Krane.initial_status.duration - # Krane.priority_resources.duration - # Krane.priority_resources.count - # Krane.apply_all.duration - # Krane.normal_resources.duration - # Krane.all_resources.duration - - # ).each do |expected_metric| - # metric = metrics.find { |m| m.name == expected_metric } - # refute_nil(metric, "Metric #{expected_metric} not emitted") - # assert_includes(metric.tags, "foo:bar", "Metric #{expected_metric} did not have custom tags") - # end - # end - - # def test_all_expected_statsd_metrics_emitted_with_essential_tags - # metrics = capture_statsd_calls(client: Krane::StatsD.client) do - # result = deploy_fixtures('hello-cloud', subset: ['configmap-data.yml'], wait: false, sha: 'test-sha') - # assert_deploy_success(result) - # end - - # assert_equal(1, metrics.count { |m| m.type == :_e }, "Expected to find one event metric") - - # %w( - # Krane.validate_configuration.duration - # Krane.discover_resources.duration - # Krane.discover_resources.count - # Krane.initial_status.duration - # Krane.validate_resources.duration - # Krane.priority_resources.duration - # Krane.priority_resources.count - # Krane.apply_all.duration - # Krane.normal_resources.duration - # Krane.sync.duration - # Krane.all_resources.duration - # ).each do |expected_metric| - # metric = metrics.find { |m| m.name == expected_metric } - # refute_nil(metric, "Metric #{expected_metric} not emitted") - # assert_includes(metric.tags, "namespace:#{@namespace}", "#{metric.name} is missing namespace tag") - # assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}", "#{metric.name} is missing context tag") - # assert_includes(metric.tags, "sha:test-sha", "#{metric.name} is missing sha tag") - # end - # end - - # def test_global_deploy_emits_expected_statsd_metrics - # metrics = capture_statsd_calls(client: Krane::StatsD.client) do - # assert_deploy_success(deploy_global_fixtures('globals')) - # end - - # assert_equal(1, metrics.count { |m| m.type == :_e }, "Expected to find one event metric") - - # %w( - # Krane.validate_configuration.duration - # Krane.discover_resources.duration - # Krane.discover_resources.count - # Krane.initial_status.duration - # Krane.validate_resources.duration - # Krane.apply_all.duration - # Krane.normal_resources.duration - # Krane.sync.duration - # Krane.all_resources.duration - # ).each do |expected_metric| - # metric = metrics.find { |m| m.name == expected_metric } - # refute_nil(metric, "Metric #{expected_metric} not emitted") - # assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}", "#{metric.name} is missing context tag") - # end - # end - - # ## BLACK BOX TESTS - # # test_global_deploy_black_box_failure is in test/integration/krane_test.rb - # # because it does not modify global state. The following two tests modify - # # global state and must be run in serially - # def test_global_deploy_black_box_success - # setup_template_dir("globals") do |target_dir| - # flags = "-f #{target_dir} --selector app=krane" - # out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") - # assert_empty(out) - # assert_match("Success", err) - # assert_predicate(status, :success?) - # end - # ensure - # build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false) - # end - - # def test_global_deploy_black_box_timeout - # setup_template_dir("globals") do |target_dir| - # flags = "-f #{target_dir} --selector app=krane --global-timeout=0.1s" - # out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") - # assert_empty(out) - # assert_match("TIMED OUT", err) - # refute_predicate(status, :success?) - # assert_equal(status.exitstatus, 70) - # end - # ensure - # build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false) - # end - - # def test_global_deploy_prune_black_box_success - # namespace_name = "test-app" - # setup_template_dir("globals") do |target_dir| - # flags = "-f #{target_dir} --selector app=krane" - # namespace_str = "apiVersion: v1\nkind: Namespace\nmetadata:\n name: #{namespace_name}"\ - # "\n labels:\n app: krane" - # File.write(File.join(target_dir, "namespace.yml"), namespace_str) - # out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") - # assert_empty(out) - # assert_match("Successfully deployed 3 resource", err) - # assert_match(/#{namespace_name}\W+Exists/, err) - # assert_match("Success", err) - # assert_predicate(status, :success?) - - # flags = "-f #{target_dir}/storage_classes.yml --selector app=krane" - # out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") - # assert_empty(out) - # refute_match(namespace_name, err) # Asserting that the namespace is not pruned - # assert_match("Pruned 1 resource and successfully deployed 1 resource", err) - # assert_predicate(status, :success?) - # end - # ensure - # build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false) - # build_kubectl.run("delete", "namespace", namespace_name, use_namespace: false, log_failure: false) - # end - - # ## TESTS THAT DEPLOY CRDS - # # Tests that create CRDs cannot be run in parallel with tests that deploy namespaced resources - # # This is because the CRD kind may torn down in the middle of the namespaced deploy, causing it to be seen - # # when we build the pruning whitelist, but gone by the time we attempt to list instances for pruning purposes. - # # When this happens, the namespaced deploy will fail with an `apply` error. - # def test_cr_merging - # assert_deploy_success(deploy_global_fixtures("crd", subset: %(mail.yml))) - # result = deploy_fixtures("crd", subset: %w(mail_cr.yml)) do |f| - # cr = f.dig("mail_cr.yml", "Mail").first - # cr["kind"] = add_unique_prefix_for_test(cr["kind"]) - # end - # assert_deploy_success(result) - - # result = deploy_fixtures("crd", subset: %w(mail_cr.yml)) do |f| - # cr = f.dig("mail_cr.yml", "Mail").first - # cr["spec"]["something"] = 5 - # cr["kind"] = add_unique_prefix_for_test(cr["kind"]) - # end - # assert_deploy_success(result) - # end - - # def test_custom_resources_predeployed - # assert_deploy_success(deploy_global_fixtures("crd", subset: %w(mail.yml things.yml widgets.yml)) do |f| - # mail = f.dig("mail.yml", "CustomResourceDefinition").first - # mail["metadata"]["annotations"] = {} - - # things = f.dig("things.yml", "CustomResourceDefinition").first - # things["metadata"]["annotations"] = { - # "krane.shopify.io/predeployed" => "true", - # } - - # widgets = f.dig("widgets.yml", "CustomResourceDefinition").first - # widgets["metadata"]["annotations"] = { - # "krane.shopify.io/predeployed" => "false", - # } - # end) - # reset_logger - - # result = deploy_fixtures("crd", subset: %w(mail_cr.yml things_cr.yml widgets_cr.yml)) do |f| - # f.each do |_filename, contents| - # contents.each do |_kind, crs| # all of the resources are CRs, so change all of them - # crs.each { |cr| cr["kind"] = add_unique_prefix_for_test(cr["kind"]) } - # end - # end - # end - # assert_deploy_success(result) - - # mail_cr_id = "#{add_unique_prefix_for_test('Mail')}/my-first-mail" - # thing_cr_id = "#{add_unique_prefix_for_test('Thing')}/my-first-thing" - # widget_cr_id = "#{add_unique_prefix_for_test('Widget')}/my-first-widget" - # assert_logs_match_all([ - # /Phase 3: Predeploying priority resources/, - # /Successfully deployed in \d.\ds: #{mail_cr_id}/, - # /Successfully deployed in \d.\ds: #{thing_cr_id}/, - # /Phase 4: Deploying all resources/, - # /Successfully deployed in \d.\ds: #{mail_cr_id}, #{thing_cr_id}, #{widget_cr_id}/, - # ], in_order: true) - # refute_logs_match( - # /Successfully deployed in \d.\ds: #{widget_cr_id}/, - # ) - # end - - # def test_cr_deploys_without_rollout_conditions_when_none_present - # assert_deploy_success(deploy_global_fixtures("crd", subset: %(widgets.yml))) - # result = deploy_fixtures("crd", subset: %w(widgets_cr.yml)) do |f| - # f.each do |_filename, contents| # all of the resources are CRs, so change all of them - # contents.each do |_kind, crs| - # crs.each { |cr| cr["kind"] = add_unique_prefix_for_test(cr["kind"]) } - # end - # end - # end - - # assert_deploy_success(result) - # prefixed_kind = add_unique_prefix_for_test("Widget") - # assert_logs_match_all([ - # "Don't know how to monitor resources of type #{prefixed_kind}.", - # "Assuming #{prefixed_kind}/my-first-widget deployed successfully.", - # %r{Widget/my-first-widget\s+Exists}, - # ]) - # end - - # def test_cr_success_with_default_rollout_conditions - # assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_default_conditions.yml))) - # success_conditions = { - # "status" => { - # "observedGeneration" => 1, - # "conditions" => [ - # { - # "type" => "Ready", - # "reason" => "test", - # "message" => "test", - # "status" => "True", - # }, - # ], - # }, - # } - - # result = deploy_fixtures("crd", subset: ["with_default_conditions_cr.yml"]) do |resource| - # cr = resource["with_default_conditions_cr.yml"]["Parameterized"].first - # cr.merge!(success_conditions) - # cr["kind"] = add_unique_prefix_for_test(cr["kind"]) - # end - # assert_deploy_success(result) - # assert_logs_match_all([ - # %r{Successfully deployed in .*: #{add_unique_prefix_for_test("Parameterized")}\/with-default-params}, - # %r{Parameterized/with-default-params\s+Healthy}, - # ]) - # end - - # def test_cr_success_with_service - # filepath = "#{fixture_path('crd')}/service_cr.yml" - # out, err, st = build_kubectl.run("create", "-f", filepath, log_failure: true, use_namespace: false) - # assert(st.success?, "Failed to create CRD: #{out}\n#{err}") - - # assert_deploy_success(deploy_fixtures("crd", subset: %w(web.yml))) - - # refute_logs_match(/Predeploying priority resources/) - # assert_logs_match_all([/Phase 3: Deploying all resources/]) - # ensure - # build_kubectl.run("delete", "-f", filepath, use_namespace: false, log_failure: false) - # end - - # def test_cr_failure_with_default_rollout_conditions - # assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_default_conditions.yml))) - # failure_conditions = { - # "status" => { - # "observedGeneration" => 1, - # "conditions" => [ - # { - # "type" => "Failed", - # "reason" => "test", - # "message" => "custom resource rollout failed", - # "status" => "True", - # }, - # ], - # }, - # } - - # result = deploy_fixtures("crd", subset: ["with_default_conditions_cr.yml"]) do |resource| - # cr = resource["with_default_conditions_cr.yml"]["Parameterized"].first - # cr.merge!(failure_conditions) - # cr["kind"] = add_unique_prefix_for_test(cr["kind"]) - # end - # assert_deploy_failure(result) - - # assert_logs_match_all([ - # "Parameterized/with-default-params: FAILED", - # "custom resource rollout failed", - # "Final status: Unhealthy", - # ], in_order: true) - # end - - # def test_cr_success_with_arbitrary_rollout_conditions - # assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_custom_conditions.yml))) - - # success_conditions = { - # "spec" => {}, - # "status" => { - # "observedGeneration" => 1, - # "test_field" => "success_value", - # "condition" => "success_value", - # }, - # } - - # result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml"]) do |resource| - # cr = resource["with_custom_conditions_cr.yml"]["Customized"].first - # cr["kind"] = add_unique_prefix_for_test(cr["kind"]) - # cr.merge!(success_conditions) - # end - # assert_deploy_success(result) - # assert_logs_match_all([ - # %r{Successfully deployed in .*: #{add_unique_prefix_for_test("Customized")}\/with-customized-params}, - # ]) - # end - - # def test_cr_failure_with_arbitrary_rollout_conditions - # assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_custom_conditions.yml))) - # cr = load_fixtures("crd", ["with_custom_conditions_cr.yml"]) - # failure_conditions = { - # "spec" => {}, - # "status" => { - # "test_field" => "failure_value", - # "error_msg" => "test error message jsonpath", - # "observedGeneration" => 1, - # "condition" => "failure_value", - # }, - # } - - # result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml"]) do |resource| - # cr = resource["with_custom_conditions_cr.yml"]["Customized"].first - # cr["kind"] = add_unique_prefix_for_test(cr["kind"]) - # cr.merge!(failure_conditions) - # end - # assert_deploy_failure(result) - # assert_logs_match_all([ - # "test error message jsonpath", - # "test custom error message", - # ]) - # end - - # def test_deploying_crs_with_invalid_crd_conditions_fails - # # Since CRDs are not always deployed along with their CRs and krane is not the only way CRDs are - # # deployed, we need to model the case where poorly configured rollout_conditions are present before deploying a CR - # fixtures = load_fixtures("crd", "with_custom_conditions.yml") - # crd = fixtures["with_custom_conditions.yml"]["CustomResourceDefinition"].first - # crd["metadata"]["annotations"].merge!(rollout_conditions_annotation_key => "blah") - # apply_scope_to_resources(fixtures, labels: "app=krane,test=#{@namespace}") - # Tempfile.open([@namespace, ".yml"]) do |f| - # f.write(YAML.dump(crd)) - # f.fsync - # @deployed_global_fixture_paths << f.path - # out, err, st = build_kubectl.run("create", "-f", f.path, log_failure: true, use_namespace: false) - # assert(st.success?, "Failed to create invalid CRD: #{out}\n#{err}") - # end - - # result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml", "with_custom_conditions_cr2.yml"]) do |f| - # f.each do |_filename, contents| - # contents.each do |_kind, crs| # all of the resources are CRs, so change all of them - # crs.each { |cr| cr["kind"] = add_unique_prefix_for_test(cr["kind"]) } - # end - # end - # end - # assert_deploy_failure(result) - # prefixed_name = add_unique_prefix_for_test("Customized-with-customized-params") - # assert_logs_match_all([ - # /Invalid template: #{prefixed_name}/, - # /Rollout conditions are not valid JSON/, - # /Invalid template: #{prefixed_name}/, - # /Rollout conditions are not valid JSON/, - # ], in_order: true) - # end - - # def test_crd_can_fail - # result = deploy_global_fixtures("crd", subset: %(mail.yml)) do |f| - # crd = f.dig("mail.yml", "CustomResourceDefinition").first - # names = crd.dig("spec", "names") - # names["listKind"] = 'Conflict' - # end - # assert_deploy_success(result) - - # second_name = add_unique_prefix_for_test("others") - # result = deploy_global_fixtures("crd", subset: %(mail.yml), prune: false) do |f| - # crd = f.dig("mail.yml", "CustomResourceDefinition").first - # names = crd.dig("spec", "names") - # names["listKind"] = "Conflict" - # names["plural"] = second_name - # crd["metadata"]["name"] = "#{second_name}.stable.example.io" - # end - # assert_deploy_failure(result) - # assert_logs_match_all([ - # "Deploying CustomResourceDefinition/#{second_name}.stable.example.io (timeout: 120s)", - # "CustomResourceDefinition/#{second_name}.stable.example.io: FAILED", - # 'Final status: ListKindConflict ("Conflict" is already in use)', - # ]) - # end - - # def test_global_deploy_validation_catches_namespaced_cr - # assert_deploy_success(deploy_global_fixtures("crd", subset: %(mail.yml))) - # reset_logger - # result = deploy_global_fixtures("crd", subset: %(mail_cr.yml)) do |fixtures| - # mail = fixtures["mail_cr.yml"]["Mail"].first - # mail["kind"] = add_unique_prefix_for_test(mail["kind"]) - # end - # assert_deploy_failure(result) - # assert_logs_match_all([ - # "Phase 1: Initializing deploy", - # "Using resource selector app=krane", - # "All required parameters and files are present", - # "Discovering resources:", - # "- #{add_unique_prefix_for_test('Mail')}/#{add_unique_prefix_for_test('my-first-mail')}", - # "Result: FAILURE", - # "This command cannot deploy namespaced resources", - # "Namespaced resources:", - # "#{add_unique_prefix_for_test('my-first-mail')} (#{add_unique_prefix_for_test('Mail')})", - # ]) - # end - - # def test_resource_discovery_stops_deploys_when_fetch_resources_kubectl_errs - # failure_msg = "Stubbed failure reason" - # Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_resources).raises(Krane::FatalKubeAPIError, failure_msg) - # assert_deploy_failure(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"])) - - # assert_logs_match_all([ - # "Result: FAILURE", - # failure_msg, - # ], in_order: true) - # end - - # def test_resource_discovery_stops_deploys_when_fetch_crds_kubectl_errs - # failure_msg = "Stubbed failure reason" - # Krane::ClusterResourceDiscovery.any_instance.expects(:crds).raises(Krane::FatalKubeAPIError, failure_msg) - # assert_deploy_failure(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"])) - - # assert_logs_match_all([ - # "Result: FAILURE", - # failure_msg, - # ], in_order: true) - # end + def test_unreachable_context + old_config = ENV['KUBECONFIG'] + begin + ENV['KUBECONFIG'] = File.join(__dir__, '../fixtures/kube-config/dummy_config.yml') + kubectl_instance = build_kubectl(timeout: '0.1s') + result = deploy_fixtures('hello-cloud', kubectl_instance: kubectl_instance) + assert_deploy_failure(result) + assert_logs_match_all([ + 'Result: FAILURE', + "Something went wrong connecting to #{TEST_CONTEXT}", + ], in_order: true) + ensure + ENV['KUBECONFIG'] = old_config + end + end + + def test_multiple_configuration_files + old_config = ENV['KUBECONFIG'] + config_file = File.join(__dir__, '../fixtures/kube-config/unknown_config.yml') + ENV['KUBECONFIG'] = config_file + result = deploy_fixtures('hello-cloud') + assert_deploy_failure(result) + assert_logs_match_all([ + 'Result: FAILURE', + 'Configuration invalid', + "Kubeconfig not found at #{config_file}", + ], in_order: true) + reset_logger + + ENV['KUBECONFIG'] = " : " + result = deploy_fixtures('hello-cloud') + assert_deploy_failure(result) + assert_logs_match_all([ + 'Result: FAILURE', + 'Configuration invalid', + "Kubeconfig file name(s) not set in $KUBECONFIG", + ], in_order: true) + reset_logger + + default_config = "#{Dir.home}/.kube/config" + extra_config = File.join(__dir__, '../fixtures/kube-config/dummy_config.yml') + ENV['KUBECONFIG'] = "#{default_config}:#{extra_config}" + result = deploy_fixtures('hello-cloud', subset: ["configmap-data.yml"]) + assert_deploy_success(result) + ensure + ENV['KUBECONFIG'] = old_config + end + + # We want to be sure that failures to apply resources with potentially sensitive output don't leak any content. + # Currently our only sensitive resource is `Secret`, but we cannot reproduce a failure scenario where the kubectl + # output contains the filename (which would trigger some extra logging). This test stubs `Deployment` to be sensitive + # to recreate such a condition + def test_apply_failure_with_sensitive_resources_hides_template_content + logger.level = 0 + Krane::Deployment.any_instance.expects(:sensitive_template_content?).returns(true).at_least_once + result = deploy_fixtures("hello-cloud", subset: ["web.yml.erb"], render_erb: true) do |fixtures| + bad_port_name = "http_test_is_really_long_and_invalid_chars" + svc = fixtures["web.yml.erb"]["Service"].first + svc["spec"]["ports"].first["targetPort"] = bad_port_name + deployment = fixtures["web.yml.erb"]["Deployment"].first + deployment["spec"]["template"]["spec"]["containers"].first["ports"].first["name"] = bad_port_name + end + assert_deploy_failure(result) + refute_logs_match(%r{Kubectl err:.*something/invalid}) + + assert_logs_match_all([ + "Command failed: apply -f", + /Invalid template: Deployment-web.*\.yml/, + ]) + + refute_logs_match("kind: Deployment") # content of the sensitive template + end + + ## METRICS TESTS + # Metrics tests must be run serially to ensure our global client isn't capturing metrics from other tests + def test_stage_related_metrics_include_custom_tags_from_namespace + hello_cloud = FixtureSetAssertions::HelloCloud.new(@namespace) + kubeclient.patch_namespace(hello_cloud.namespace, metadata: { labels: { foo: 'bar' } }) + metrics = capture_statsd_calls(client: Krane::StatsD.client) do + assert_deploy_success(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"], wait: false)) + end + + %w( + Krane.validate_configuration.duration + Krane.discover_resources.duration + Krane.discover_resources.count + Krane.validate_resources.duration + Krane.initial_status.duration + Krane.priority_resources.duration + Krane.priority_resources.count + Krane.apply_all.duration + Krane.normal_resources.duration + Krane.all_resources.duration + + ).each do |expected_metric| + metric = metrics.find { |m| m.name == expected_metric } + refute_nil(metric, "Metric #{expected_metric} not emitted") + assert_includes(metric.tags, "foo:bar", "Metric #{expected_metric} did not have custom tags") + end + end + + def test_all_expected_statsd_metrics_emitted_with_essential_tags + metrics = capture_statsd_calls(client: Krane::StatsD.client) do + result = deploy_fixtures('hello-cloud', subset: ['configmap-data.yml'], wait: false, sha: 'test-sha') + assert_deploy_success(result) + end + + assert_equal(1, metrics.count { |m| m.type == :_e }, "Expected to find one event metric") + + %w( + Krane.validate_configuration.duration + Krane.discover_resources.duration + Krane.discover_resources.count + Krane.initial_status.duration + Krane.validate_resources.duration + Krane.priority_resources.duration + Krane.priority_resources.count + Krane.apply_all.duration + Krane.normal_resources.duration + Krane.sync.duration + Krane.all_resources.duration + ).each do |expected_metric| + metric = metrics.find { |m| m.name == expected_metric } + refute_nil(metric, "Metric #{expected_metric} not emitted") + assert_includes(metric.tags, "namespace:#{@namespace}", "#{metric.name} is missing namespace tag") + assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}", "#{metric.name} is missing context tag") + assert_includes(metric.tags, "sha:test-sha", "#{metric.name} is missing sha tag") + end + end + + def test_global_deploy_emits_expected_statsd_metrics + metrics = capture_statsd_calls(client: Krane::StatsD.client) do + assert_deploy_success(deploy_global_fixtures('globals')) + end + + assert_equal(1, metrics.count { |m| m.type == :_e }, "Expected to find one event metric") + + %w( + Krane.validate_configuration.duration + Krane.discover_resources.duration + Krane.discover_resources.count + Krane.initial_status.duration + Krane.validate_resources.duration + Krane.apply_all.duration + Krane.normal_resources.duration + Krane.sync.duration + Krane.all_resources.duration + ).each do |expected_metric| + metric = metrics.find { |m| m.name == expected_metric } + refute_nil(metric, "Metric #{expected_metric} not emitted") + assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}", "#{metric.name} is missing context tag") + end + end + + ## BLACK BOX TESTS + # test_global_deploy_black_box_failure is in test/integration/krane_test.rb + # because it does not modify global state. The following two tests modify + # global state and must be run in serially + def test_global_deploy_black_box_success + setup_template_dir("globals") do |target_dir| + flags = "-f #{target_dir} --selector app=krane" + out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") + assert_empty(out) + assert_match("Success", err) + assert_predicate(status, :success?) + end + ensure + build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false) + end + + def test_global_deploy_black_box_timeout + setup_template_dir("globals") do |target_dir| + flags = "-f #{target_dir} --selector app=krane --global-timeout=0.1s" + out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") + assert_empty(out) + assert_match("TIMED OUT", err) + refute_predicate(status, :success?) + assert_equal(status.exitstatus, 70) + end + ensure + build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false) + end + + def test_global_deploy_prune_black_box_success + namespace_name = "test-app" + setup_template_dir("globals") do |target_dir| + flags = "-f #{target_dir} --selector app=krane" + namespace_str = "apiVersion: v1\nkind: Namespace\nmetadata:\n name: #{namespace_name}"\ + "\n labels:\n app: krane" + File.write(File.join(target_dir, "namespace.yml"), namespace_str) + out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") + assert_empty(out) + assert_match("Successfully deployed 3 resource", err) + assert_match(/#{namespace_name}\W+Exists/, err) + assert_match("Success", err) + assert_predicate(status, :success?) + + flags = "-f #{target_dir}/storage_classes.yml --selector app=krane" + out, err, status = krane_black_box("global-deploy", "#{KubeclientHelper::TEST_CONTEXT} #{flags}") + assert_empty(out) + refute_match(namespace_name, err) # Asserting that the namespace is not pruned + assert_match("Pruned 1 resource and successfully deployed 1 resource", err) + assert_predicate(status, :success?) + end + ensure + build_kubectl.run("delete", "-f", fixture_path("globals"), use_namespace: false, log_failure: false) + build_kubectl.run("delete", "namespace", namespace_name, use_namespace: false, log_failure: false) + end + + ## TESTS THAT DEPLOY CRDS + # Tests that create CRDs cannot be run in parallel with tests that deploy namespaced resources + # This is because the CRD kind may torn down in the middle of the namespaced deploy, causing it to be seen + # when we build the pruning whitelist, but gone by the time we attempt to list instances for pruning purposes. + # When this happens, the namespaced deploy will fail with an `apply` error. + def test_cr_merging + assert_deploy_success(deploy_global_fixtures("crd", subset: %(mail.yml))) + result = deploy_fixtures("crd", subset: %w(mail_cr.yml)) do |f| + cr = f.dig("mail_cr.yml", "Mail").first + cr["kind"] = add_unique_prefix_for_test(cr["kind"]) + end + assert_deploy_success(result) + + result = deploy_fixtures("crd", subset: %w(mail_cr.yml)) do |f| + cr = f.dig("mail_cr.yml", "Mail").first + cr["spec"]["something"] = 5 + cr["kind"] = add_unique_prefix_for_test(cr["kind"]) + end + assert_deploy_success(result) + end + + def test_custom_resources_predeployed + assert_deploy_success(deploy_global_fixtures("crd", subset: %w(mail.yml things.yml widgets.yml)) do |f| + mail = f.dig("mail.yml", "CustomResourceDefinition").first + mail["metadata"]["annotations"] = {} + + things = f.dig("things.yml", "CustomResourceDefinition").first + things["metadata"]["annotations"] = { + "krane.shopify.io/predeployed" => "true", + } + + widgets = f.dig("widgets.yml", "CustomResourceDefinition").first + widgets["metadata"]["annotations"] = { + "krane.shopify.io/predeployed" => "false", + } + end) + reset_logger + + result = deploy_fixtures("crd", subset: %w(mail_cr.yml things_cr.yml widgets_cr.yml)) do |f| + f.each do |_filename, contents| + contents.each do |_kind, crs| # all of the resources are CRs, so change all of them + crs.each { |cr| cr["kind"] = add_unique_prefix_for_test(cr["kind"]) } + end + end + end + assert_deploy_success(result) + + mail_cr_id = "#{add_unique_prefix_for_test('Mail')}/my-first-mail" + thing_cr_id = "#{add_unique_prefix_for_test('Thing')}/my-first-thing" + widget_cr_id = "#{add_unique_prefix_for_test('Widget')}/my-first-widget" + assert_logs_match_all([ + /Phase 3: Predeploying priority resources/, + /Successfully deployed in \d.\ds: #{mail_cr_id}/, + /Successfully deployed in \d.\ds: #{thing_cr_id}/, + /Phase 4: Deploying all resources/, + /Successfully deployed in \d.\ds: #{mail_cr_id}, #{thing_cr_id}, #{widget_cr_id}/, + ], in_order: true) + refute_logs_match( + /Successfully deployed in \d.\ds: #{widget_cr_id}/, + ) + end + + def test_cr_deploys_without_rollout_conditions_when_none_present + assert_deploy_success(deploy_global_fixtures("crd", subset: %(widgets.yml))) + result = deploy_fixtures("crd", subset: %w(widgets_cr.yml)) do |f| + f.each do |_filename, contents| # all of the resources are CRs, so change all of them + contents.each do |_kind, crs| + crs.each { |cr| cr["kind"] = add_unique_prefix_for_test(cr["kind"]) } + end + end + end + + assert_deploy_success(result) + prefixed_kind = add_unique_prefix_for_test("Widget") + assert_logs_match_all([ + "Don't know how to monitor resources of type #{prefixed_kind}.", + "Assuming #{prefixed_kind}/my-first-widget deployed successfully.", + %r{Widget/my-first-widget\s+Exists}, + ]) + end + + def test_cr_success_with_default_rollout_conditions + assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_default_conditions.yml))) + success_conditions = { + "status" => { + "observedGeneration" => 1, + "conditions" => [ + { + "type" => "Ready", + "reason" => "test", + "message" => "test", + "status" => "True", + }, + ], + }, + } + + result = deploy_fixtures("crd", subset: ["with_default_conditions_cr.yml"]) do |resource| + cr = resource["with_default_conditions_cr.yml"]["Parameterized"].first + cr.merge!(success_conditions) + cr["kind"] = add_unique_prefix_for_test(cr["kind"]) + end + assert_deploy_success(result) + assert_logs_match_all([ + %r{Successfully deployed in .*: #{add_unique_prefix_for_test("Parameterized")}\/with-default-params}, + %r{Parameterized/with-default-params\s+Healthy}, + ]) + end + + def test_cr_success_with_service + filepath = "#{fixture_path('crd')}/service_cr.yml" + out, err, st = build_kubectl.run("create", "-f", filepath, log_failure: true, use_namespace: false) + assert(st.success?, "Failed to create CRD: #{out}\n#{err}") + + assert_deploy_success(deploy_fixtures("crd", subset: %w(web.yml))) + + refute_logs_match(/Predeploying priority resources/) + assert_logs_match_all([/Phase 3: Deploying all resources/]) + ensure + build_kubectl.run("delete", "-f", filepath, use_namespace: false, log_failure: false) + end + + def test_cr_failure_with_default_rollout_conditions + assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_default_conditions.yml))) + failure_conditions = { + "status" => { + "observedGeneration" => 1, + "conditions" => [ + { + "type" => "Failed", + "reason" => "test", + "message" => "custom resource rollout failed", + "status" => "True", + }, + ], + }, + } + + result = deploy_fixtures("crd", subset: ["with_default_conditions_cr.yml"]) do |resource| + cr = resource["with_default_conditions_cr.yml"]["Parameterized"].first + cr.merge!(failure_conditions) + cr["kind"] = add_unique_prefix_for_test(cr["kind"]) + end + assert_deploy_failure(result) + + assert_logs_match_all([ + "Parameterized/with-default-params: FAILED", + "custom resource rollout failed", + "Final status: Unhealthy", + ], in_order: true) + end + + def test_cr_success_with_arbitrary_rollout_conditions + assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_custom_conditions.yml))) + + success_conditions = { + "spec" => {}, + "status" => { + "observedGeneration" => 1, + "test_field" => "success_value", + "condition" => "success_value", + }, + } + + result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml"]) do |resource| + cr = resource["with_custom_conditions_cr.yml"]["Customized"].first + cr["kind"] = add_unique_prefix_for_test(cr["kind"]) + cr.merge!(success_conditions) + end + assert_deploy_success(result) + assert_logs_match_all([ + %r{Successfully deployed in .*: #{add_unique_prefix_for_test("Customized")}\/with-customized-params}, + ]) + end + + def test_cr_failure_with_arbitrary_rollout_conditions + assert_deploy_success(deploy_global_fixtures("crd", subset: %(with_custom_conditions.yml))) + cr = load_fixtures("crd", ["with_custom_conditions_cr.yml"]) + failure_conditions = { + "spec" => {}, + "status" => { + "test_field" => "failure_value", + "error_msg" => "test error message jsonpath", + "observedGeneration" => 1, + "condition" => "failure_value", + }, + } + + result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml"]) do |resource| + cr = resource["with_custom_conditions_cr.yml"]["Customized"].first + cr["kind"] = add_unique_prefix_for_test(cr["kind"]) + cr.merge!(failure_conditions) + end + assert_deploy_failure(result) + assert_logs_match_all([ + "test error message jsonpath", + "test custom error message", + ]) + end + + def test_deploying_crs_with_invalid_crd_conditions_fails + # Since CRDs are not always deployed along with their CRs and krane is not the only way CRDs are + # deployed, we need to model the case where poorly configured rollout_conditions are present before deploying a CR + fixtures = load_fixtures("crd", "with_custom_conditions.yml") + crd = fixtures["with_custom_conditions.yml"]["CustomResourceDefinition"].first + crd["metadata"]["annotations"].merge!(rollout_conditions_annotation_key => "blah") + apply_scope_to_resources(fixtures, labels: "app=krane,test=#{@namespace}") + Tempfile.open([@namespace, ".yml"]) do |f| + f.write(YAML.dump(crd)) + f.fsync + @deployed_global_fixture_paths << f.path + out, err, st = build_kubectl.run("create", "-f", f.path, log_failure: true, use_namespace: false) + assert(st.success?, "Failed to create invalid CRD: #{out}\n#{err}") + end + + result = deploy_fixtures("crd", subset: ["with_custom_conditions_cr.yml", "with_custom_conditions_cr2.yml"]) do |f| + f.each do |_filename, contents| + contents.each do |_kind, crs| # all of the resources are CRs, so change all of them + crs.each { |cr| cr["kind"] = add_unique_prefix_for_test(cr["kind"]) } + end + end + end + assert_deploy_failure(result) + prefixed_name = add_unique_prefix_for_test("Customized-with-customized-params") + assert_logs_match_all([ + /Invalid template: #{prefixed_name}/, + /Rollout conditions are not valid JSON/, + /Invalid template: #{prefixed_name}/, + /Rollout conditions are not valid JSON/, + ], in_order: true) + end + + def test_crd_can_fail + result = deploy_global_fixtures("crd", subset: %(mail.yml)) do |f| + crd = f.dig("mail.yml", "CustomResourceDefinition").first + names = crd.dig("spec", "names") + names["listKind"] = 'Conflict' + end + assert_deploy_success(result) + + second_name = add_unique_prefix_for_test("others") + result = deploy_global_fixtures("crd", subset: %(mail.yml), prune: false) do |f| + crd = f.dig("mail.yml", "CustomResourceDefinition").first + names = crd.dig("spec", "names") + names["listKind"] = "Conflict" + names["plural"] = second_name + crd["metadata"]["name"] = "#{second_name}.stable.example.io" + end + assert_deploy_failure(result) + assert_logs_match_all([ + "Deploying CustomResourceDefinition/#{second_name}.stable.example.io (timeout: 120s)", + "CustomResourceDefinition/#{second_name}.stable.example.io: FAILED", + 'Final status: ListKindConflict ("Conflict" is already in use)', + ]) + end + + def test_global_deploy_validation_catches_namespaced_cr + assert_deploy_success(deploy_global_fixtures("crd", subset: %(mail.yml))) + reset_logger + result = deploy_global_fixtures("crd", subset: %(mail_cr.yml)) do |fixtures| + mail = fixtures["mail_cr.yml"]["Mail"].first + mail["kind"] = add_unique_prefix_for_test(mail["kind"]) + end + assert_deploy_failure(result) + assert_logs_match_all([ + "Phase 1: Initializing deploy", + "Using resource selector app=krane", + "All required parameters and files are present", + "Discovering resources:", + "- #{add_unique_prefix_for_test('Mail')}/#{add_unique_prefix_for_test('my-first-mail')}", + "Result: FAILURE", + "This command cannot deploy namespaced resources", + "Namespaced resources:", + "#{add_unique_prefix_for_test('my-first-mail')} (#{add_unique_prefix_for_test('Mail')})", + ]) + end + + def test_resource_discovery_stops_deploys_when_fetch_resources_kubectl_errs + failure_msg = "Stubbed failure reason" + Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_resources).raises(Krane::FatalKubeAPIError, failure_msg) + assert_deploy_failure(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"])) + + assert_logs_match_all([ + "Result: FAILURE", + failure_msg, + ], in_order: true) + end + + def test_resource_discovery_stops_deploys_when_fetch_crds_kubectl_errs + failure_msg = "Stubbed failure reason" + Krane::ClusterResourceDiscovery.any_instance.expects(:crds).raises(Krane::FatalKubeAPIError, failure_msg) + assert_deploy_failure(deploy_fixtures("hello-cloud", subset: ["configmap-data.yml"])) + + assert_logs_match_all([ + "Result: FAILURE", + failure_msg, + ], in_order: true) + end + def test_batch_dry_run_apply_failure_falls_back_to_individual_resource_dry_run_validation Krane::KubernetesResource.any_instance.expects(:validate_definition).with do |kwargs| @@ -525,18 +526,10 @@ def test_batch_dry_run_apply_failure_falls_back_to_individual_resource_dry_run_v end def test_batch_dry_run_apply_success_precludes_individual_resource_dry_run_validation - Krane::KubernetesResource.any_instance.expects(:validate_definition).with do |kwargs| - kwargs[:kubectl].nil? && !kwargs[:dry_run] - end + Krane::KubernetesResource.any_instance.expects(:validate_definition).times(0) deploy_fixtures("hello-cloud", subset: %w(secret.yml)) end - - # def test_resources_with_side_effect_inducing_webhooks_are_not_server_side_dry_run - # Krane::Kubectl.any_instance.expects(:run).with("get", "mutatingwebhookconfigurations", "-ojson", - # use_namespace: false, raise_if_not_found: true).returns({}) - - # end - # end + end private diff --git a/test/integration-serial/serial_task_run_test.rb b/test/integration-serial/serial_task_run_test.rb index c05eda4d5..cb67eba57 100644 --- a/test/integration-serial/serial_task_run_test.rb +++ b/test/integration-serial/serial_task_run_test.rb @@ -6,79 +6,79 @@ class SerialTaskRunTest < Krane::IntegrationTest include StatsD::Instrument::Assertions # Mocha is not thread-safe: https://github.com/freerange/mocha#thread-safety - # def test_run_without_verify_result_fails_if_pod_was_not_created - # deploy_task_template - # task_runner = build_task_runner + def test_run_without_verify_result_fails_if_pod_was_not_created + deploy_task_template + task_runner = build_task_runner - # # Sketchy, but stubbing the kubeclient doesn't work (and wouldn't be concurrency-friendly) - # # Finding a way to reliably trigger a create failure would be much better, if possible - # mock = mock() - # template = kubeclient.get_pod_template('hello-cloud-template-runner', @namespace) - # mock.expects(:get_pod_template).returns(template) - # mock.expects(:create_pod).raises(Kubeclient::HttpError.new("409", "Pod with same name exists", {})) - # task_runner.instance_variable_set(:@kubeclient, mock) + # Sketchy, but stubbing the kubeclient doesn't work (and wouldn't be concurrency-friendly) + # Finding a way to reliably trigger a create failure would be much better, if possible + mock = mock() + template = kubeclient.get_pod_template('hello-cloud-template-runner', @namespace) + mock.expects(:get_pod_template).returns(template) + mock.expects(:create_pod).raises(Kubeclient::HttpError.new("409", "Pod with same name exists", {})) + task_runner.instance_variable_set(:@kubeclient, mock) - # result = task_runner.run(**run_params(verify_result: false)) - # assert_task_run_failure(result) + result = task_runner.run(**run_params(verify_result: false)) + assert_task_run_failure(result) - # assert_logs_match_all([ - # "Running pod", - # "Result: FAILURE", - # "Failed to create pod", - # "Kubeclient::HttpError: Pod with same name exists", - # ], in_order: true) - # end + assert_logs_match_all([ + "Running pod", + "Result: FAILURE", + "Failed to create pod", + "Kubeclient::HttpError: Pod with same name exists", + ], in_order: true) + end - # # Run statsd tests in serial because capture_statsd_calls modifies global state in a way - # # that makes capturing metrics across parallel runs unreliable - # def test_failure_statsd_metric_emitted - # bad_ns = "missing" - # task_runner = build_task_runner(ns: bad_ns) + # Run statsd tests in serial because capture_statsd_calls modifies global state in a way + # that makes capturing metrics across parallel runs unreliable + def test_failure_statsd_metric_emitted + bad_ns = "missing" + task_runner = build_task_runner(ns: bad_ns) - # metrics = capture_statsd_calls(client: Krane::StatsD.client) do - # result = task_runner.run(**run_params) - # assert_task_run_failure(result) - # end + metrics = capture_statsd_calls(client: Krane::StatsD.client) do + result = task_runner.run(**run_params) + assert_task_run_failure(result) + end - # metric = metrics.find do |m| - # m.name == "Krane.task_runner.duration" && m.tags.include?("namespace:#{bad_ns}") - # end - # assert(metric, "No result metric found for this test") - # assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}") - # assert_includes(metric.tags, "status:failure") - # end + metric = metrics.find do |m| + m.name == "Krane.task_runner.duration" && m.tags.include?("namespace:#{bad_ns}") + end + assert(metric, "No result metric found for this test") + assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}") + assert_includes(metric.tags, "status:failure") + end - # def test_success_statsd_metric_emitted - # deploy_task_template - # task_runner = build_task_runner + def test_success_statsd_metric_emitted + deploy_task_template + task_runner = build_task_runner - # metrics = capture_statsd_calls(client: Krane::StatsD.client) do - # result = task_runner.run(**run_params.merge(verify_result: false)) - # assert_task_run_success(result) - # end + metrics = capture_statsd_calls(client: Krane::StatsD.client) do + result = task_runner.run(**run_params.merge(verify_result: false)) + assert_task_run_success(result) + end - # metric = metrics.find do |m| - # m.name == "Krane.task_runner.duration" && m.tags.include?("namespace:#{@namespace}") - # end - # assert(metric, "No result metric found for this test") - # assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}") - # assert_includes(metric.tags, "status:success") - # end + metric = metrics.find do |m| + m.name == "Krane.task_runner.duration" && m.tags.include?("namespace:#{@namespace}") + end + assert(metric, "No result metric found for this test") + assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}") + assert_includes(metric.tags, "status:success") + end - # def test_timedout_statsd_metric_emitted - # deploy_task_template - # task_runner = build_task_runner(global_timeout: 0) + def test_timedout_statsd_metric_emitted + deploy_task_template + task_runner = build_task_runner(global_timeout: 0) - # metrics = capture_statsd_calls(client: Krane::StatsD.client) do - # result = task_runner.run(**run_params.merge(arguments: ["sleep 5"])) - # assert_task_run_failure(result, :timed_out) - # end + metrics = capture_statsd_calls(client: Krane::StatsD.client) do + result = task_runner.run(**run_params.merge(arguments: ["sleep 5"])) + assert_task_run_failure(result, :timed_out) + end - # metric = metrics.find do |m| - # m.name == "Krane.task_runner.duration" && m.tags.include?("namespace:#{@namespace}") - # end - # assert(metric, "No result metric found for this test") - # assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}") - # assert_includes(metric.tags, "status:timeout") - # end + metric = metrics.find do |m| + m.name == "Krane.task_runner.duration" && m.tags.include?("namespace:#{@namespace}") + end + assert(metric, "No result metric found for this test") + assert_includes(metric.tags, "context:#{KubeclientHelper::TEST_CONTEXT}") + assert_includes(metric.tags, "status:timeout") + end end From 911dc8ac5632764f17f704d1f3359e00016068e8 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Tue, 2 Feb 2021 16:01:53 -0500 Subject: [PATCH 03/27] refinement --- lib/krane/cluster_resource_discovery.rb | 10 +++ lib/krane/deploy_task.rb | 10 +-- .../for_serial_deploy_tests/ingress_hook.json | 71 +++++++++++++++++++ .../for_serial_deploy_tests/ingress_hook.yml | 48 ------------- .../for_serial_deploy_tests/secret_hook.json | 71 +++++++++++++++++++ test/integration-serial/serial_deploy_test.rb | 31 ++++++-- 6 files changed, 182 insertions(+), 59 deletions(-) create mode 100644 test/fixtures/for_serial_deploy_tests/ingress_hook.json delete mode 100644 test/fixtures/for_serial_deploy_tests/ingress_hook.yml create mode 100644 test/fixtures/for_serial_deploy_tests/secret_hook.json diff --git a/lib/krane/cluster_resource_discovery.rb b/lib/krane/cluster_resource_discovery.rb index 80c01477e..bb7989b44 100644 --- a/lib/krane/cluster_resource_discovery.rb +++ b/lib/krane/cluster_resource_discovery.rb @@ -61,6 +61,16 @@ def fetch_resources(namespaced: false) end end + def fetch_mutating_webhook_configurations + command = %w(get mutatingwebhookconfigurations) + raw_json, err, st = kubectl.run(*command, output: "json", attempts: 5, use_namespace: false) + if st.success? + JSON.parse(raw_json)["items"] + else + raise FatalKubeAPIError, "Error retrieving mutatingwebhookconfigurations: #{err}" + end + end + private # kubectl api-versions returns a list of group/version strings e.g. autoscaling/v2beta2 diff --git a/lib/krane/deploy_task.rb b/lib/krane/deploy_task.rb index 44dc4bcdf..5b4023d92 100644 --- a/lib/krane/deploy_task.rb +++ b/lib/krane/deploy_task.rb @@ -279,11 +279,7 @@ def validate_configuration(prune:) def partition_dry_run_resources(resources) individuals = [] - out, err, st = kubectl.run("get", "mutatingwebhookconfigurations", "-ojson", - use_namespace: false, raise_if_not_found: true) - raise FatalDeploymentError, "error retrieving mutatingwebhookconfigurations: #{err.message}" unless st.success? - - mutating_webhooks = JSON.parse(out).dig("items") + mutating_webhooks = cluster_resource_discoverer.fetch_mutating_webhook_configurations mutating_webhooks.each do |spec| spec.dig('webhooks').each do |webhook| match_policy = webhook.dig('matchPolicy') @@ -297,8 +293,8 @@ def partition_dry_run_resources(resources) kinds.each do |kind| individuals += resources.select do |r| (r.group == group || group == '*' || match_policy == "Equivalent") && - (r.version == version || version == '*' || match_policy == "Equivalent") && - (r.type.downcase == kind.downcase) + (r.version == version || version == '*' || match_policy == "Equivalent") && + (r.type.downcase == kind.downcase) end resources -= individuals end diff --git a/test/fixtures/for_serial_deploy_tests/ingress_hook.json b/test/fixtures/for_serial_deploy_tests/ingress_hook.json new file mode 100644 index 000000000..2cbe2882e --- /dev/null +++ b/test/fixtures/for_serial_deploy_tests/ingress_hook.json @@ -0,0 +1,71 @@ +{ + "apiVersion": "v1", + "items": [ + { + "apiVersion": "admissionregistration.k8s.io/v1", + "kind": "MutatingWebhookConfiguration", + "metadata": { + "creationTimestamp": "2020-08-04T19:49:44Z", + "generation": 2, + "name": "oauthboss-webhook-configuration", + "resourceVersion": "3115431", + "selfLink": "/apis/admissionregistration.k8s.io/v1/mutatingwebhookconfigurations/oauthboss-webhook-configuration", + "uid": "7afce875-5175-430d-95aa-de8ec97f15b0" + }, + "webhooks": [ + { + "admissionReviewVersions": [ + "v1beta1" + ], + "clientConfig": { + "caBundle": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUMwakNDQWJxZ0F3SUJBZ0lCQURBTkJna3Foa2lHOXcwQkFRc0ZBREFhTVJnd0ZnWURWUVFERXc5M1pXSm8KYjI5ckxXTmxjblF0WTJFd0hoY05NakF3T0RBME1UazBPVFExV2hjTk16QXdPREF5TVRrME9UUTFXakFhTVJndwpGZ1lEVlFRREV3OTNaV0pvYjI5ckxXTmxjblF0WTJFd2dnRWlNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUJEd0F3CmdnRUtBb0lCQVFDOGs1MjNobEFsald4RitITk8wWk9qQTZ4MGNSa1RQVHdkZm9nK3JGOXZ3Y21XRTBFT3kyYlUKZ3BNRU5SQzVpVzRxS1pCYUJ6MEU1bXcvbWNjcGNrbXR1V05pODVML0hzUjVlbWR2aWdUamJYb0RMeWxEdjNMLwpRSlVWOVRIYWFhaXh5cmFMNWhIRUw4bTBhTFNZcWc5MktxODJnWXB0aTB5azBnU1BqdjhUTHBFazhvdWp2L01YCnVJZWtQQzZOTWFDaU1BY1FBTVRyc1NiSmZqL0x0WGpwWk9FM2xCNWpTazNDUW1sT0t5Mm8zaDd6WTZ1RFNtN28KY3orMkhiUXhybCt1SkcrY1B4WVdoWXZReXpLWGViWU11ajd5Wm5FRmI2WFc3dC9ROVo2VmFzMi9DS1Z4Yll4LwppdXEvbU1xZk4xZnNvUitJTmVXVlJ3YlJ0dUJ5VkljRkFnTUJBQUdqSXpBaE1BNEdBMVVkRHdFQi93UUVBd0lDCnBEQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRQXc4VWQwOWRYR09kQkwKYnQ5M3JuYk41VzBkMkJUZk43OXJ6SVpadHlmMjREQVQ1SGc1QVV2Qmc4QjJNaFRqdU50VlN6dzN4bGV3TDk3aQo0Wk9JWGJPL3hZTWp3OXRibldDY2o0MlVKRnJUOEN2eVZEekExeS9VVE1lMFpyU3lOUGpWTlBaMmUrb3IvRFdkCmRtbmg0THZ6c3VNbVBPZk96NmUwZmljMzdtLzZjL3JZV0l6aTJYWWRwSllHa1J1eW51UlVUbDZpUjBOa0xXaGYKc25aNll6TnFXT2YvNi9xdHduNkhVckttYWN3L2Q3blBNSDRpQTFId1MyZFh6WkxVR3NVZkZIREFkU2FVSWR3cApsZDFUUjErQjVyMnhiUW95eVo4YnhuTk5Ud04rU2dFZGc5MjJKUFlaRkZFWldDL1NXOWVYcUZnTWM3RlQyMHh4ClE1NW1qdHRICi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K", + "service": { + "name": "oauthboss", + "namespace": "cloudbosses", + "path": "/oauthboss", + "port": 443 + } + }, + "failurePolicy": "Ignore", + "matchPolicy": "Exact", + "name": "oauthboss.cloudbosses.admission.online.shopify.com", + "namespaceSelector": { + "matchExpressions": [ + { + "key": "control-plane", + "operator": "DoesNotExist" + } + ] + }, + "objectSelector": {}, + "reinvocationPolicy": "Never", + "rules": [ + { + "apiGroups": [ + "extensions" + ], + "apiVersions": [ + "v1beta1" + ], + "operations": [ + "CREATE", + "UPDATE" + ], + "resources": [ + "ingresses" + ], + "scope": "*" + } + ], + "sideEffects": "Unknown", + "timeoutSeconds": 30 + } + ] + } + ], + "kind": "List", + "metadata": { + "resourceVersion": "", + "selfLink": "" + } +} \ No newline at end of file diff --git a/test/fixtures/for_serial_deploy_tests/ingress_hook.yml b/test/fixtures/for_serial_deploy_tests/ingress_hook.yml deleted file mode 100644 index 4fba2ea5d..000000000 --- a/test/fixtures/for_serial_deploy_tests/ingress_hook.yml +++ /dev/null @@ -1,48 +0,0 @@ ---- -apiVersion: v1 -items: -- apiVersion: admissionregistration.k8s.io/v1 - kind: MutatingWebhookConfiguration - metadata: - creationTimestamp: '2020-08-04T19:49:44Z' - generation: 2 - name: oauthboss-webhook-configuration - resourceVersion: '3115431' - selfLink: "/apis/admissionregistration.k8s.io/v1/mutatingwebhookconfigurations/oauthboss-webhook-configuration" - uid: 7afce875-5175-430d-95aa-de8ec97f15b0 - webhooks: - - admissionReviewVersions: - - v1beta1 - clientConfig: - caBundle: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUMwakNDQWJxZ0F3SUJBZ0lCQURBTkJna3Foa2lHOXcwQkFRc0ZBREFhTVJnd0ZnWURWUVFERXc5M1pXSm8KYjI5ckxXTmxjblF0WTJFd0hoY05NakF3T0RBME1UazBPVFExV2hjTk16QXdPREF5TVRrME9UUTFXakFhTVJndwpGZ1lEVlFRREV3OTNaV0pvYjI5ckxXTmxjblF0WTJFd2dnRWlNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUJEd0F3CmdnRUtBb0lCQVFDOGs1MjNobEFsald4RitITk8wWk9qQTZ4MGNSa1RQVHdkZm9nK3JGOXZ3Y21XRTBFT3kyYlUKZ3BNRU5SQzVpVzRxS1pCYUJ6MEU1bXcvbWNjcGNrbXR1V05pODVML0hzUjVlbWR2aWdUamJYb0RMeWxEdjNMLwpRSlVWOVRIYWFhaXh5cmFMNWhIRUw4bTBhTFNZcWc5MktxODJnWXB0aTB5azBnU1BqdjhUTHBFazhvdWp2L01YCnVJZWtQQzZOTWFDaU1BY1FBTVRyc1NiSmZqL0x0WGpwWk9FM2xCNWpTazNDUW1sT0t5Mm8zaDd6WTZ1RFNtN28KY3orMkhiUXhybCt1SkcrY1B4WVdoWXZReXpLWGViWU11ajd5Wm5FRmI2WFc3dC9ROVo2VmFzMi9DS1Z4Yll4LwppdXEvbU1xZk4xZnNvUitJTmVXVlJ3YlJ0dUJ5VkljRkFnTUJBQUdqSXpBaE1BNEdBMVVkRHdFQi93UUVBd0lDCnBEQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRQXc4VWQwOWRYR09kQkwKYnQ5M3JuYk41VzBkMkJUZk43OXJ6SVpadHlmMjREQVQ1SGc1QVV2Qmc4QjJNaFRqdU50VlN6dzN4bGV3TDk3aQo0Wk9JWGJPL3hZTWp3OXRibldDY2o0MlVKRnJUOEN2eVZEekExeS9VVE1lMFpyU3lOUGpWTlBaMmUrb3IvRFdkCmRtbmg0THZ6c3VNbVBPZk96NmUwZmljMzdtLzZjL3JZV0l6aTJYWWRwSllHa1J1eW51UlVUbDZpUjBOa0xXaGYKc25aNll6TnFXT2YvNi9xdHduNkhVckttYWN3L2Q3blBNSDRpQTFId1MyZFh6WkxVR3NVZkZIREFkU2FVSWR3cApsZDFUUjErQjVyMnhiUW95eVo4YnhuTk5Ud04rU2dFZGc5MjJKUFlaRkZFWldDL1NXOWVYcUZnTWM3RlQyMHh4ClE1NW1qdHRICi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K - service: - name: oauthboss - namespace: cloudbosses - path: "/oauthboss" - port: 443 - failurePolicy: Ignore - matchPolicy: Exact - name: oauthboss.cloudbosses.admission.online.shopify.com - namespaceSelector: - matchExpressions: - - key: control-plane - operator: DoesNotExist - objectSelector: {} - reinvocationPolicy: Never - rules: - - apiGroups: - - extensions - apiVersions: - - v1beta1 - operations: - - CREATE - - UPDATE - resources: - - ingresses - scope: "*" - sideEffects: Unknown - timeoutSeconds: 30 -kind: List -metadata: - resourceVersion: '' - selfLink: '' diff --git a/test/fixtures/for_serial_deploy_tests/secret_hook.json b/test/fixtures/for_serial_deploy_tests/secret_hook.json new file mode 100644 index 000000000..0c012c205 --- /dev/null +++ b/test/fixtures/for_serial_deploy_tests/secret_hook.json @@ -0,0 +1,71 @@ +{ + "apiVersion": "v1", + "items": [ + { + "apiVersion": "admissionregistration.k8s.io/v1", + "kind": "MutatingWebhookConfiguration", + "metadata": { + "creationTimestamp": "2020-08-04T19:49:44Z", + "generation": 2, + "name": "oauthboss-webhook-configuration", + "resourceVersion": "3115431", + "selfLink": "/apis/admissionregistration.k8s.io/v1/mutatingwebhookconfigurations/oauthboss-webhook-configuration", + "uid": "7afce875-5175-430d-95aa-de8ec97f15b0" + }, + "webhooks": [ + { + "admissionReviewVersions": [ + "v1beta1" + ], + "clientConfig": { + "caBundle": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUMwakNDQWJxZ0F3SUJBZ0lCQURBTkJna3Foa2lHOXcwQkFRc0ZBREFhTVJnd0ZnWURWUVFERXc5M1pXSm8KYjI5ckxXTmxjblF0WTJFd0hoY05NakF3T0RBME1UazBPVFExV2hjTk16QXdPREF5TVRrME9UUTFXakFhTVJndwpGZ1lEVlFRREV3OTNaV0pvYjI5ckxXTmxjblF0WTJFd2dnRWlNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUJEd0F3CmdnRUtBb0lCQVFDOGs1MjNobEFsald4RitITk8wWk9qQTZ4MGNSa1RQVHdkZm9nK3JGOXZ3Y21XRTBFT3kyYlUKZ3BNRU5SQzVpVzRxS1pCYUJ6MEU1bXcvbWNjcGNrbXR1V05pODVML0hzUjVlbWR2aWdUamJYb0RMeWxEdjNMLwpRSlVWOVRIYWFhaXh5cmFMNWhIRUw4bTBhTFNZcWc5MktxODJnWXB0aTB5azBnU1BqdjhUTHBFazhvdWp2L01YCnVJZWtQQzZOTWFDaU1BY1FBTVRyc1NiSmZqL0x0WGpwWk9FM2xCNWpTazNDUW1sT0t5Mm8zaDd6WTZ1RFNtN28KY3orMkhiUXhybCt1SkcrY1B4WVdoWXZReXpLWGViWU11ajd5Wm5FRmI2WFc3dC9ROVo2VmFzMi9DS1Z4Yll4LwppdXEvbU1xZk4xZnNvUitJTmVXVlJ3YlJ0dUJ5VkljRkFnTUJBQUdqSXpBaE1BNEdBMVVkRHdFQi93UUVBd0lDCnBEQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRQXc4VWQwOWRYR09kQkwKYnQ5M3JuYk41VzBkMkJUZk43OXJ6SVpadHlmMjREQVQ1SGc1QVV2Qmc4QjJNaFRqdU50VlN6dzN4bGV3TDk3aQo0Wk9JWGJPL3hZTWp3OXRibldDY2o0MlVKRnJUOEN2eVZEekExeS9VVE1lMFpyU3lOUGpWTlBaMmUrb3IvRFdkCmRtbmg0THZ6c3VNbVBPZk96NmUwZmljMzdtLzZjL3JZV0l6aTJYWWRwSllHa1J1eW51UlVUbDZpUjBOa0xXaGYKc25aNll6TnFXT2YvNi9xdHduNkhVckttYWN3L2Q3blBNSDRpQTFId1MyZFh6WkxVR3NVZkZIREFkU2FVSWR3cApsZDFUUjErQjVyMnhiUW95eVo4YnhuTk5Ud04rU2dFZGc5MjJKUFlaRkZFWldDL1NXOWVYcUZnTWM3RlQyMHh4ClE1NW1qdHRICi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K", + "service": { + "name": "oauthboss", + "namespace": "cloudbosses", + "path": "/oauthboss", + "port": 443 + } + }, + "failurePolicy": "Ignore", + "matchPolicy": "Equivalent", + "name": "oauthboss.cloudbosses.admission.online.shopify.com", + "namespaceSelector": { + "matchExpressions": [ + { + "key": "control-plane", + "operator": "DoesNotExist" + } + ] + }, + "objectSelector": {}, + "reinvocationPolicy": "Never", + "rules": [ + { + "apiGroups": [ + "core" + ], + "apiVersions": [ + "v1" + ], + "operations": [ + "CREATE", + "UPDATE" + ], + "resources": [ + "secrets" + ], + "scope": "*" + } + ], + "sideEffects": "Unknown", + "timeoutSeconds": 30 + } + ] + } + ], + "kind": "List", + "metadata": { + "resourceVersion": "", + "selfLink": "" + } +} diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index 2da319a2f..2c375976e 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -3,9 +3,9 @@ class SerialDeployTest < Krane::IntegrationTest include StatsD::Instrument::Assertions - ## GLOBAL CONTEXT MANIPULATION TESTS - # This can be run in parallel if we allow passing the config file path to DeployTask.new - # See https://github.com/Shopify/krane/pull/428#pullrequestreview-209720675 + # GLOBAL CONTEXT MANIPULATION TESTS + This can be run in parallel if we allow passing the config file path to DeployTask.new + See https://github.com/Shopify/krane/pull/428#pullrequestreview-209720675 def test_unreachable_context old_config = ENV['KUBECONFIG'] begin @@ -514,7 +514,6 @@ def test_resource_discovery_stops_deploys_when_fetch_crds_kubectl_errs ], in_order: true) end - def test_batch_dry_run_apply_failure_falls_back_to_individual_resource_dry_run_validation Krane::KubernetesResource.any_instance.expects(:validate_definition).with do |kwargs| kwargs[:kubectl].is_a?(Krane::Kubectl) && kwargs[:dry_run] @@ -529,6 +528,30 @@ def test_batch_dry_run_apply_success_precludes_individual_resource_dry_run_valid Krane::KubernetesResource.any_instance.expects(:validate_definition).times(0) deploy_fixtures("hello-cloud", subset: %w(secret.yml)) end + + def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_side_dry_run + resp = JSON.parse(File.read(File.join(fixture_path("for_serial_deploy_tests"), "ingress_hook.json")))["items"] + Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) + Krane::ResourceDeployer.any_instance.expects(:dry_run).with do |params| + params.length == 2 && (params.map(&:type) - ["Deployment", "Service"]).empty? + end + Krane::Ingress.any_instance.expects(:validate_definition) + result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb), render_erb: true) + end + + def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency_fail_batch_running + resp = JSON.parse(File.read(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")))["items"] + Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) + Krane::KubernetesResource.any_instance.expects(:validate_definition).times(4) # resources we expect to have to individually validate + deploy_fixtures('hello-cloud', subset: %w(web.yml.erb secret.yml), render_erb: true) do |fixtures| + container = fixtures['web.yml.erb']['Deployment'][0]['spec']['template']['spec']['containers'][0] + container['volumes'] = [ + name: 'secret', + secret: { + secretName: fixtures['secret.yml']["Secret"][0]['metadata']['name'], + }, + ] + end end private From 3713af89562acc42ac381d3acb505315257be94f Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Tue, 2 Feb 2021 16:11:48 -0500 Subject: [PATCH 04/27] test fix --- test/integration-serial/serial_deploy_test.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index 2c375976e..614b0141e 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -539,18 +539,18 @@ def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_sid result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb), render_erb: true) end - def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency_fail_batch_running + def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency_does_not_fail_batch_running resp = JSON.parse(File.read(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")))["items"] Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) - Krane::KubernetesResource.any_instance.expects(:validate_definition).times(4) # resources we expect to have to individually validate + Krane::KubernetesResource.any_instance.expects(:validate_definition).times(1) # Only secret should call this deploy_fixtures('hello-cloud', subset: %w(web.yml.erb secret.yml), render_erb: true) do |fixtures| - container = fixtures['web.yml.erb']['Deployment'][0]['spec']['template']['spec']['containers'][0] - container['volumes'] = [ - name: 'secret', - secret: { - secretName: fixtures['secret.yml']["Secret"][0]['metadata']['name'], + container = fixtures['web.yml.erb']['Deployment'][0]['spec']['template']['spec'] + container['volumes'] = [{ + 'name' => 'secret', + 'secret' => { + 'secretName' => fixtures['secret.yml']["Secret"][0]['metadata']['name'], }, - ] + }] end end From a63e5d27bb9bf1679e9ae68fbc10bcbd901916df Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Tue, 2 Feb 2021 16:13:42 -0500 Subject: [PATCH 05/27] readd comment --- test/integration-serial/serial_deploy_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index 614b0141e..9b43a30bd 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -3,9 +3,9 @@ class SerialDeployTest < Krane::IntegrationTest include StatsD::Instrument::Assertions - # GLOBAL CONTEXT MANIPULATION TESTS - This can be run in parallel if we allow passing the config file path to DeployTask.new - See https://github.com/Shopify/krane/pull/428#pullrequestreview-209720675 + ## GLOBAL CONTEXT MANIPULATION TESTS + # This can be run in parallel if we allow passing the config file path to DeployTask.new + # See https://github.com/Shopify/krane/pull/428#pullrequestreview-209720675 def test_unreachable_context old_config = ENV['KUBECONFIG'] begin From 7062e274554d16318ae9547f858fe68208446125 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Tue, 2 Feb 2021 16:16:20 -0500 Subject: [PATCH 06/27] cleanup --- lib/krane/kubernetes_resource/config_map.rb | 4 ++++ lib/krane/resource_deployer.rb | 1 + test/integration-serial/serial_deploy_test.rb | 4 +++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/krane/kubernetes_resource/config_map.rb b/lib/krane/kubernetes_resource/config_map.rb index 098f48050..47d5dec64 100644 --- a/lib/krane/kubernetes_resource/config_map.rb +++ b/lib/krane/kubernetes_resource/config_map.rb @@ -7,6 +7,10 @@ def deploy_succeeded? exists? end + def status + exists? "Available" : "Not Found" + end + def deploy_failed? false end diff --git a/lib/krane/resource_deployer.rb b/lib/krane/resource_deployer.rb index 5efbb5b35..569356692 100644 --- a/lib/krane/resource_deployer.rb +++ b/lib/krane/resource_deployer.rb @@ -161,6 +161,7 @@ def apply_all(resources, prune, dry_run: false) global_mode = resources.all?(&:global?) out, err, st = kubectl.run(*command, log_failure: false, output_is_sensitive: output_is_sensitive, attempts: 2, use_namespace: !global_mode) + tags = statsd_tags + (dry_run ? ['dry_run:true'] : ['dry_run:false']) Krane::StatsD.client.distribution('apply_all.duration', Krane::StatsD.duration(start), tags: tags) if st.success? diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index 9b43a30bd..8b6c1bf6c 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -537,13 +537,14 @@ def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_sid end Krane::Ingress.any_instance.expects(:validate_definition) result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb), render_erb: true) + assert_deploy_success(result) end def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency_does_not_fail_batch_running resp = JSON.parse(File.read(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")))["items"] Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) Krane::KubernetesResource.any_instance.expects(:validate_definition).times(1) # Only secret should call this - deploy_fixtures('hello-cloud', subset: %w(web.yml.erb secret.yml), render_erb: true) do |fixtures| + result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb secret.yml), render_erb: true) do |fixtures| container = fixtures['web.yml.erb']['Deployment'][0]['spec']['template']['spec'] container['volumes'] = [{ 'name' => 'secret', @@ -552,6 +553,7 @@ def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency }, }] end + assert_deploy_success(result) end private From a9b9bc4f797adbd5c530487236fb923bba077955 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Tue, 2 Feb 2021 16:16:45 -0500 Subject: [PATCH 07/27] more fix --- lib/krane/kubernetes_resource/config_map.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/krane/kubernetes_resource/config_map.rb b/lib/krane/kubernetes_resource/config_map.rb index 47d5dec64..6b9d232fd 100644 --- a/lib/krane/kubernetes_resource/config_map.rb +++ b/lib/krane/kubernetes_resource/config_map.rb @@ -8,7 +8,7 @@ def deploy_succeeded? end def status - exists? "Available" : "Not Found" + exists ? "Available" : "Not Found" end def deploy_failed? From f120b8ee6e547fa65b1187289ab037d17c6a8caa Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Tue, 2 Feb 2021 16:17:18 -0500 Subject: [PATCH 08/27] grr --- lib/krane/kubernetes_resource/config_map.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/krane/kubernetes_resource/config_map.rb b/lib/krane/kubernetes_resource/config_map.rb index 6b9d232fd..364656922 100644 --- a/lib/krane/kubernetes_resource/config_map.rb +++ b/lib/krane/kubernetes_resource/config_map.rb @@ -8,7 +8,7 @@ def deploy_succeeded? end def status - exists ? "Available" : "Not Found" + exists? ? "Available" : "Not Found" end def deploy_failed? From eb50b0e6d7e14c407a3fcf184c76e1328fad4eca Mon Sep 17 00:00:00 2001 From: Timothy Smith <31742287+timothysmith0609@users.noreply.github.com> Date: Wed, 3 Feb 2021 10:35:10 -0500 Subject: [PATCH 09/27] Update lib/krane/kubernetes_resource.rb Co-authored-by: Danny Turner --- lib/krane/kubernetes_resource.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/krane/kubernetes_resource.rb b/lib/krane/kubernetes_resource.rb index c68d685c5..5934ecf41 100644 --- a/lib/krane/kubernetes_resource.rb +++ b/lib/krane/kubernetes_resource.rb @@ -228,7 +228,7 @@ def group def version prefix, version = @definition.dig("apiVersion").split("/") - version ? version : prefix + version || prefix end def kubectl_resource_type From a79eb6687d539bc49e9807732793254de9c4de35 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Wed, 3 Feb 2021 11:07:24 -0500 Subject: [PATCH 10/27] dup resources to avoid mutating issues --- lib/krane/deploy_task.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/krane/deploy_task.rb b/lib/krane/deploy_task.rb index 5b4023d92..76c788aac 100644 --- a/lib/krane/deploy_task.rb +++ b/lib/krane/deploy_task.rb @@ -308,7 +308,7 @@ def partition_dry_run_resources(resources) def validate_resources(resources) validate_globals(resources) - batchable_resources, individuals = partition_dry_run_resources(resources) + batchable_resources, individuals = partition_dry_run_resources(resources.dup) batch_dry_run_success = kubectl.server_dry_run_enabled? && validate_dry_run(batchable_resources) individuals += batchable_resources unless batch_dry_run_success Krane::Concurrency.split_across_threads(individuals) do |r| From 78d2b1df8cfdce32ac1998ab2ab5bfd3df00a3ec Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Wed, 3 Feb 2021 11:07:29 -0500 Subject: [PATCH 11/27] fix test --- test/integration-serial/serial_deploy_test.rb | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index 8b6c1bf6c..36ef23454 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -526,25 +526,35 @@ def test_batch_dry_run_apply_failure_falls_back_to_individual_resource_dry_run_v def test_batch_dry_run_apply_success_precludes_individual_resource_dry_run_validation Krane::KubernetesResource.any_instance.expects(:validate_definition).times(0) - deploy_fixtures("hello-cloud", subset: %w(secret.yml)) + result = deploy_fixtures("hello-cloud", subset: %w(secret.yml)) + assert_deploy_success(result) + assert_logs_match_all([ + "Result: SUCCESS", + "Successfully deployed 1 resource", + ], in_order: true) end def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_side_dry_run resp = JSON.parse(File.read(File.join(fixture_path("for_serial_deploy_tests"), "ingress_hook.json")))["items"] Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) Krane::ResourceDeployer.any_instance.expects(:dry_run).with do |params| - params.length == 2 && (params.map(&:type) - ["Deployment", "Service"]).empty? + params.length == 3 && (params.map(&:type) - ["Deployment", "Service", "ConfigMap"]).empty? end Krane::Ingress.any_instance.expects(:validate_definition) - result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb), render_erb: true) + result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb configmap-data.yml), render_erb: true) assert_deploy_success(result) + assert_logs_match_all([ + "Result: SUCCESS", + "Successfully deployed 4 resources", + ], in_order: true) end def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency_does_not_fail_batch_running resp = JSON.parse(File.read(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")))["items"] Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) Krane::KubernetesResource.any_instance.expects(:validate_definition).times(1) # Only secret should call this - result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb secret.yml), render_erb: true) do |fixtures| + result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb secret.yml configmap-data.yml), + render_erb: true) do |fixtures| container = fixtures['web.yml.erb']['Deployment'][0]['spec']['template']['spec'] container['volumes'] = [{ 'name' => 'secret', @@ -554,6 +564,10 @@ def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency }] end assert_deploy_success(result) + assert_logs_match_all([ + "Result: SUCCESS", + "Successfully deployed 5 resources", + ], in_order: true) end private From 3c2e91e827c22279916cd8b3189e184b1346c22e Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Wed, 3 Feb 2021 11:09:08 -0500 Subject: [PATCH 12/27] empty commit From 893d6c30b5d2782b830338e7dd94b7780b06d1bf Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Wed, 3 Feb 2021 12:25:03 -0500 Subject: [PATCH 13/27] object-oriented MutatingWebhookConfiguration" --- lib/krane/cluster_resource_discovery.rb | 5 +- lib/krane/deploy_task.rb | 26 ++---- .../mutating_webhook_configuration.rb | 83 +++++++++++++++++++ test/integration-serial/serial_deploy_test.rb | 11 ++- 4 files changed, 101 insertions(+), 24 deletions(-) create mode 100644 lib/krane/kubernetes_resource/mutating_webhook_configuration.rb diff --git a/lib/krane/cluster_resource_discovery.rb b/lib/krane/cluster_resource_discovery.rb index bb7989b44..b2bb86d03 100644 --- a/lib/krane/cluster_resource_discovery.rb +++ b/lib/krane/cluster_resource_discovery.rb @@ -65,7 +65,10 @@ def fetch_mutating_webhook_configurations command = %w(get mutatingwebhookconfigurations) raw_json, err, st = kubectl.run(*command, output: "json", attempts: 5, use_namespace: false) if st.success? - JSON.parse(raw_json)["items"] + JSON.parse(raw_json)["items"].map do |definition| + KubernetesResource::MutatingWebhookConfiguration.new(namespace: namespace, context: context, logger: logger, + definition: definition, statsd_tags: @namespace_tags) + end else raise FatalKubeAPIError, "Error retrieving mutatingwebhookconfigurations: #{err}" end diff --git a/lib/krane/deploy_task.rb b/lib/krane/deploy_task.rb index 76c788aac..0e1c21e02 100644 --- a/lib/krane/deploy_task.rb +++ b/lib/krane/deploy_task.rb @@ -30,6 +30,7 @@ custom_resource_definition horizontal_pod_autoscaler secret + mutating_webhook_configuration ).each do |subresource| require "krane/kubernetes_resource/#{subresource}" end @@ -280,27 +281,10 @@ def validate_configuration(prune:) def partition_dry_run_resources(resources) individuals = [] mutating_webhooks = cluster_resource_discoverer.fetch_mutating_webhook_configurations - mutating_webhooks.each do |spec| - spec.dig('webhooks').each do |webhook| - match_policy = webhook.dig('matchPolicy') - webhook.dig('rules').each do |rule| - next if %w(None NoneOnDryRun).include?(rule.dig('sideEffects')) - groups = rule.dig('apiGroups') - versions = rule.dig('apiVersions') - kinds = rule.dig('resources').map(&:singularize) - groups.each do |group| - versions.each do |version| - kinds.each do |kind| - individuals += resources.select do |r| - (r.group == group || group == '*' || match_policy == "Equivalent") && - (r.version == version || version == '*' || match_policy == "Equivalent") && - (r.type.downcase == kind.downcase) - end - resources -= individuals - end - end - end - end + mutating_webhooks.each do |mutating_webhook| + mutating_webhook.webhooks.each do |webhook| + individuals = resources.select { |resource| webhook.matches_resource?(resource) } + resources -= individuals end end [resources, individuals] diff --git a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb new file mode 100644 index 000000000..53a0885d5 --- /dev/null +++ b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module Krane + class MutatingWebhookConfiguration < KubernetesResource + class Webhook + EQUIVALENT = 'Equivalent' + EXACT = 'Exact' + class Rule + def initialize(definition) + @definition = definition + end + + def matches_resource?(resource, accept_equivalent:) + groups.each do |group| + versions.each do |version| + resources.each do |kind| + return true if (resource.group == group || group == '*' || accept_equivalent) && + (resource.version == version || version == '*' || accept_equivalent) && + (resource.type.downcase == kind.downcase.singularize) + end + end + end + false + end + + def groups + @definition.dig('apiGroups') + end + + def versions + @definition.dig('apiVersions') + end + + def resources + @definition.dig('resources') + end + end + + def initialize(definition) + @definition = definition + end + + def side_effects + @definition.dig('sideEffects') + end + + def has_side_effects? + !%w(None NoneOnDryRun).include?(side_effects) + end + + def match_policy + @definition.dig('matchPolicy') + end + + def matches_resource?(resource, skip_rule_if_side_effect_none: true) + return false if skip_rule_if_side_effect_none && !has_side_effects? + rules.any? do |rule| + rule.matches_resource?(resource, accept_equivalent: match_policy == EQUIVALENT) + end + end + + def rules + (@definition.dig('rules') || []).map { |rule| Rule.new(rule) } + end + end + + def initialize(namespace:, context:, definition:, logger:, statsd_tags:) + @webhooks = (definition.dig('webhooks') || []).map { |hook| Webhook.new(hook) } + super(namespace: namespace, context: context, definition: definition, + logger: logger, statsd_tags: statsd_tags) + end + + TIMEOUT = 30.seconds + + def deploy_succeeded? + exists? + end + + def webhooks + (@definition.dig('webhooks') || []).map { |webhook| Webhook.new(webhook) } + end + end +end diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index 36ef23454..3023255ca 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -535,7 +535,7 @@ def test_batch_dry_run_apply_success_precludes_individual_resource_dry_run_valid end def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_side_dry_run - resp = JSON.parse(File.read(File.join(fixture_path("for_serial_deploy_tests"), "ingress_hook.json")))["items"] + resp = mutating_webhook_fixture(File.join(fixture_path("for_serial_deploy_tests"), "ingress_hook.json")) Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) Krane::ResourceDeployer.any_instance.expects(:dry_run).with do |params| params.length == 3 && (params.map(&:type) - ["Deployment", "Service", "ConfigMap"]).empty? @@ -550,7 +550,7 @@ def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_sid end def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency_does_not_fail_batch_running - resp = JSON.parse(File.read(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")))["items"] + resp = mutating_webhook_fixture(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")) Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) Krane::KubernetesResource.any_instance.expects(:validate_definition).times(1) # Only secret should call this result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb secret.yml configmap-data.yml), @@ -575,4 +575,11 @@ def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency def rollout_conditions_annotation_key Krane::Annotation.for(Krane::CustomResourceDefinition::ROLLOUT_CONDITIONS_ANNOTATION) end + + def mutating_webhook_fixture(path) + JSON.parse(File.read(path))['items'].map do |definition| + Krane::MutatingWebhookConfiguration.new(namespace: @namespace, context: @context, logger: @logger, + definition: definition, statsd_tags: @namespace_tags) + end + end end From 4d9c65c8070a5f6286f84e48f3e29b160942e265 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Wed, 3 Feb 2021 12:52:22 -0500 Subject: [PATCH 14/27] still need to validate all definitions, but not necessarily dry run --- lib/krane/deploy_task.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/krane/deploy_task.rb b/lib/krane/deploy_task.rb index 0e1c21e02..0afc48e67 100644 --- a/lib/krane/deploy_task.rb +++ b/lib/krane/deploy_task.rb @@ -295,8 +295,8 @@ def validate_resources(resources) batchable_resources, individuals = partition_dry_run_resources(resources.dup) batch_dry_run_success = kubectl.server_dry_run_enabled? && validate_dry_run(batchable_resources) individuals += batchable_resources unless batch_dry_run_success - Krane::Concurrency.split_across_threads(individuals) do |r| - r.validate_definition(kubectl: kubectl, selector: @selector, dry_run: true) + Krane::Concurrency.split_across_threads(resources) do |r| + r.validate_definition(kubectl: kubectl, selector: @selector, dry_run: individuals.include?(r)) end failed_resources = resources.select(&:validation_failed?) if failed_resources.present? From 53dbcf358ecba23309e01f822f0cd8853ad0fd50 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Wed, 3 Feb 2021 13:39:45 -0500 Subject: [PATCH 15/27] Fix test --- test/integration-serial/serial_deploy_test.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index 3023255ca..e5a36b2d2 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -525,7 +525,7 @@ def test_batch_dry_run_apply_failure_falls_back_to_individual_resource_dry_run_v end def test_batch_dry_run_apply_success_precludes_individual_resource_dry_run_validation - Krane::KubernetesResource.any_instance.expects(:validate_definition).times(0) + Krane::KubernetesResource.any_instance.expects(:validate_definition).with { |params| params[:dry_run] == false } result = deploy_fixtures("hello-cloud", subset: %w(secret.yml)) assert_deploy_success(result) assert_logs_match_all([ @@ -552,7 +552,11 @@ def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_sid def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency_does_not_fail_batch_running resp = mutating_webhook_fixture(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")) Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) - Krane::KubernetesResource.any_instance.expects(:validate_definition).times(1) # Only secret should call this + expected_dry_runs = 0 + Krane::KubernetesResource.any_instance.expects(:validate_definition).with do |params| + expected_dry_runs += 1 if params[:dry_run] + true + end.times(5) result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb secret.yml configmap-data.yml), render_erb: true) do |fixtures| container = fixtures['web.yml.erb']['Deployment'][0]['spec']['template']['spec'] @@ -564,6 +568,7 @@ def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency }] end assert_deploy_success(result) + assert_equal(expected_dry_runs, 1) assert_logs_match_all([ "Result: SUCCESS", "Successfully deployed 5 resources", From 8ed6125742fd1e6745bb62e73e047d7135327c17 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Wed, 3 Feb 2021 15:05:32 -0500 Subject: [PATCH 16/27] MutatingWebhookConfiguration unit test --- .../mutating_webhook_configuration.rb | 2 +- .../mutating_webhook_configuration_test.rb | 70 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb diff --git a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb index 53a0885d5..6d922b275 100644 --- a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb +++ b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb @@ -4,7 +4,7 @@ module Krane class MutatingWebhookConfiguration < KubernetesResource class Webhook EQUIVALENT = 'Equivalent' - EXACT = 'Exact' + class Rule def initialize(definition) @definition = definition diff --git a/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb b/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb new file mode 100644 index 000000000..cede41a60 --- /dev/null +++ b/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true +require 'test_helper' + +class MutatingWebhookConfigurationTest < Krane::TestCase + def test_load_from_json + definition = JSON.parse( + File.read(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")) + )["items"][0] + webhook_configuration = Krane::MutatingWebhookConfiguration.new( + namespace: 'test', context: 'nope', definition: definition, + logger: @logger, statsd_tags: nil + ) + assert_equal(webhook_configuration.webhooks.length, 1) + + raw_webhook = definition.dig('webhooks', 0) + webhook = webhook_configuration.webhooks.first + assert_equal(raw_webhook.dig('matchPolicy'), webhook.match_policy) + assert_equal(raw_webhook.dig('sideEffects'), webhook.side_effects) + + assert_equal(webhook.rules.length, 1) + raw_rule = definition.dig('webhooks', 0, 'rules', 0) + rule = webhook.rules.first + assert_equal(raw_rule.dig('apiGroups'), ['core']) + assert_equal(rule.dig('apiGroups'), ['core']) + + assert_equal(rule.dig('apiVersions'), ['v1']) + assert_equal(raw_rule.dig('apiVersions'), ['v1']) + + assert_equal(raw_rule.dig('resources'), ['secrets']) + assert_equal(rule.dig('resources'), ['secrets']) + end + + def test_webhook_configuration_matches_when_side_effects + secret_def = YAML.load_file(File.join(fixture_path('hello-cloud'), 'secret.yml')) + secret = Krane::Secret.new(namespace: 'test', context: 'nope', definition: secret_def, + logger: @logger, statsd_tags: nil) + + definition = JSON.parse( + File.read(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")) + )["items"][0] + webhook_configuration = Krane::MutatingWebhookConfiguration.new( + namespace: 'test', context: 'nope', definition: definition, + logger: @logger, statsd_tags: nil + ) + webhook = webhook_configuration.webhooks.first + assert(webhook.has_side_effects?) + assert(webhook.matches_resource?(secret)) + assert(webhook.matches_resource?(secret, skip_rule_if_side_effect_none: true)) + assert(webhook.matches_resource?(secret, skip_rule_if_side_effect_none: false)) + end + + def test_matches_webhook_configuration_doesnt_match_when_no_side_effects_and_flag + secret_def = YAML.load_file(File.join(fixture_path('hello-cloud'), 'secret.yml')) + secret = Krane::Secret.new(namespace: 'test', context: 'nope', definition: secret_def, + logger: @logger, statsd_tags: nil) + + definition = JSON.parse( + File.read(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")) + )["items"][0] + webhook_configuration = Krane::MutatingWebhookConfiguration.new( + namespace: 'test', context: 'nope', definition: definition, + logger: @logger, statsd_tags: nil + ) + webhook = webhook_configuration.webhooks.first + webhook.stubs(:has_side_effects?).returns(false).at_least_once + refute(webhook.matches_resource?(secret)) + refute(webhook.matches_resource?(secret, skip_rule_if_side_effect_none: true)) + assert(webhook.matches_resource?(secret, skip_rule_if_side_effect_none: false)) + end +end From cb38c180ac322fb5ece73a41f549bffe5cc034f6 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Wed, 3 Feb 2021 15:12:43 -0500 Subject: [PATCH 17/27] test fix --- .../mutating_webhook_configuration_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb b/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb index cede41a60..6b1727a5f 100644 --- a/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb +++ b/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb @@ -21,13 +21,13 @@ def test_load_from_json raw_rule = definition.dig('webhooks', 0, 'rules', 0) rule = webhook.rules.first assert_equal(raw_rule.dig('apiGroups'), ['core']) - assert_equal(rule.dig('apiGroups'), ['core']) + assert_equal(rule.groups, ['core']) - assert_equal(rule.dig('apiVersions'), ['v1']) assert_equal(raw_rule.dig('apiVersions'), ['v1']) + assert_equal(rule.versions, ['v1']) assert_equal(raw_rule.dig('resources'), ['secrets']) - assert_equal(rule.dig('resources'), ['secrets']) + assert_equal(rule.resources, ['secrets']) end def test_webhook_configuration_matches_when_side_effects From df9dc68a237d36da2db24704052acf27a3ae7849 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Thu, 4 Feb 2021 09:16:21 -0500 Subject: [PATCH 18/27] File organization + test for EXACT matching --- .../mutating_webhook_configuration.rb | 1 + .../ingress_hook.json | 0 .../secret_hook.json | 0 test/integration-serial/serial_deploy_test.rb | 4 +-- .../mutating_webhook_configuration_test.rb | 30 +++++++++++++++++-- 5 files changed, 30 insertions(+), 5 deletions(-) rename test/fixtures/{for_serial_deploy_tests => mutating_webhook_configurations}/ingress_hook.json (100%) rename test/fixtures/{for_serial_deploy_tests => mutating_webhook_configurations}/secret_hook.json (100%) diff --git a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb index 6d922b275..b9fef8f82 100644 --- a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb +++ b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb @@ -4,6 +4,7 @@ module Krane class MutatingWebhookConfiguration < KubernetesResource class Webhook EQUIVALENT = 'Equivalent' + EXACT = 'Exact' class Rule def initialize(definition) diff --git a/test/fixtures/for_serial_deploy_tests/ingress_hook.json b/test/fixtures/mutating_webhook_configurations/ingress_hook.json similarity index 100% rename from test/fixtures/for_serial_deploy_tests/ingress_hook.json rename to test/fixtures/mutating_webhook_configurations/ingress_hook.json diff --git a/test/fixtures/for_serial_deploy_tests/secret_hook.json b/test/fixtures/mutating_webhook_configurations/secret_hook.json similarity index 100% rename from test/fixtures/for_serial_deploy_tests/secret_hook.json rename to test/fixtures/mutating_webhook_configurations/secret_hook.json diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index e5a36b2d2..49300fbbb 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -535,7 +535,7 @@ def test_batch_dry_run_apply_success_precludes_individual_resource_dry_run_valid end def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_side_dry_run - resp = mutating_webhook_fixture(File.join(fixture_path("for_serial_deploy_tests"), "ingress_hook.json")) + resp = mutating_webhook_fixture(File.join(fixture_path("mutating_webhook_configurations"), "ingress_hook.json")) Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) Krane::ResourceDeployer.any_instance.expects(:dry_run).with do |params| params.length == 3 && (params.map(&:type) - ["Deployment", "Service", "ConfigMap"]).empty? @@ -550,7 +550,7 @@ def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_sid end def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency_does_not_fail_batch_running - resp = mutating_webhook_fixture(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")) + resp = mutating_webhook_fixture(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.json")) Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) expected_dry_runs = 0 Krane::KubernetesResource.any_instance.expects(:validate_definition).with do |params| diff --git a/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb b/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb index 6b1727a5f..927cdfdbf 100644 --- a/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb +++ b/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb @@ -4,7 +4,7 @@ class MutatingWebhookConfigurationTest < Krane::TestCase def test_load_from_json definition = JSON.parse( - File.read(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")) + File.read(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.json")) )["items"][0] webhook_configuration = Krane::MutatingWebhookConfiguration.new( namespace: 'test', context: 'nope', definition: definition, @@ -36,7 +36,7 @@ def test_webhook_configuration_matches_when_side_effects logger: @logger, statsd_tags: nil) definition = JSON.parse( - File.read(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")) + File.read(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.json")) )["items"][0] webhook_configuration = Krane::MutatingWebhookConfiguration.new( namespace: 'test', context: 'nope', definition: definition, @@ -55,7 +55,7 @@ def test_matches_webhook_configuration_doesnt_match_when_no_side_effects_and_fla logger: @logger, statsd_tags: nil) definition = JSON.parse( - File.read(File.join(fixture_path("for_serial_deploy_tests"), "secret_hook.json")) + File.read(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.json")) )["items"][0] webhook_configuration = Krane::MutatingWebhookConfiguration.new( namespace: 'test', context: 'nope', definition: definition, @@ -67,4 +67,28 @@ def test_matches_webhook_configuration_doesnt_match_when_no_side_effects_and_fla refute(webhook.matches_resource?(secret, skip_rule_if_side_effect_none: true)) assert(webhook.matches_resource?(secret, skip_rule_if_side_effect_none: false)) end + + def test_no_match_when_policy_is_exact_and_resource_doesnt_match + secret_def = YAML.load_file(File.join(fixture_path('hello-cloud'), 'secret.yml')) + secret = Krane::Secret.new(namespace: 'test', context: 'nope', definition: secret_def, + logger: @logger, statsd_tags: nil) + + definition = JSON.parse( + File.read(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.json")) + )["items"][0] + webhook_configuration = Krane::MutatingWebhookConfiguration.new( + namespace: 'test', context: 'nope', definition: definition, + logger: @logger, statsd_tags: nil + ) + + webhook = webhook_configuration.webhooks.first + assert(webhook.matches_resource?(secret)) + webhook.expects(:match_policy).returns(Krane::MutatingWebhookConfiguration::Webhook::EXACT).at_least(1) + assert(webhook.matches_resource?(secret)) + secret.expects(:group).returns('fake').once + refute(webhook.matches_resource?(secret)) + secret.unstub(:group) + secret.expects(:type).returns('fake').once + refute(webhook.matches_resource?(secret)) + end end From d0adc5892e14bb1bbf7fd933ef41fe118898046e Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Fri, 5 Feb 2021 10:41:04 -0500 Subject: [PATCH 19/27] map domain name objects more closely + newline --- lib/krane/deploy_task.rb | 6 +++--- .../mutating_webhook_configurations/ingress_hook.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/krane/deploy_task.rb b/lib/krane/deploy_task.rb index 0afc48e67..03aadf877 100644 --- a/lib/krane/deploy_task.rb +++ b/lib/krane/deploy_task.rb @@ -280,9 +280,9 @@ def validate_configuration(prune:) def partition_dry_run_resources(resources) individuals = [] - mutating_webhooks = cluster_resource_discoverer.fetch_mutating_webhook_configurations - mutating_webhooks.each do |mutating_webhook| - mutating_webhook.webhooks.each do |webhook| + mutating_webhook_configurations = cluster_resource_discoverer.fetch_mutating_webhook_configurations + mutating_webhook_configurations.each do |mutating_webhook_configuration| + mutating_webhook_configuration.webhooks.each do |webhook| individuals = resources.select { |resource| webhook.matches_resource?(resource) } resources -= individuals end diff --git a/test/fixtures/mutating_webhook_configurations/ingress_hook.json b/test/fixtures/mutating_webhook_configurations/ingress_hook.json index 2cbe2882e..874d497bb 100644 --- a/test/fixtures/mutating_webhook_configurations/ingress_hook.json +++ b/test/fixtures/mutating_webhook_configurations/ingress_hook.json @@ -68,4 +68,4 @@ "resourceVersion": "", "selfLink": "" } -} \ No newline at end of file +} From d464370b9dc655c0a83da669906aa71ab9f4835f Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Mon, 8 Feb 2021 09:21:28 -0500 Subject: [PATCH 20/27] resources field may also be wildcard --- lib/krane/kubernetes_resource/mutating_webhook_configuration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb index b9fef8f82..7927de21e 100644 --- a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb +++ b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb @@ -17,7 +17,7 @@ def matches_resource?(resource, accept_equivalent:) resources.each do |kind| return true if (resource.group == group || group == '*' || accept_equivalent) && (resource.version == version || version == '*' || accept_equivalent) && - (resource.type.downcase == kind.downcase.singularize) + (resource.type.downcase == kind.downcase.singularize || kind == "*") end end end From f787151e4a067dfad0b623c885d7ba73c0734959 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Mon, 8 Feb 2021 09:46:07 -0500 Subject: [PATCH 21/27] typo --- .../kubernetes_resource/mutating_webhook_configuration.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb index 7927de21e..02cf68b2e 100644 --- a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb +++ b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb @@ -61,7 +61,7 @@ def matches_resource?(resource, skip_rule_if_side_effect_none: true) end def rules - (@definition.dig('rules') || []).map { |rule| Rule.new(rule) } + @definition.fetch('rules', []).map { |rule| Rule.new(rule) } end end @@ -78,7 +78,7 @@ def deploy_succeeded? end def webhooks - (@definition.dig('webhooks') || []).map { |webhook| Webhook.new(webhook) } + @definition.fetch('webhooks', []).map { |webhook| Webhook.new(webhook) } end end end From deb2738dff16fc27ff0d50a0fdcb5971371221ed Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Mon, 8 Feb 2021 10:43:40 -0500 Subject: [PATCH 22/27] Use actual webhookconfiguration in-cluster for tests + fixes --- lib/krane/cluster_resource_discovery.rb | 2 +- .../mutating_webhook_configuration.rb | 3 + .../ingress_hook.json | 71 ------------------- .../ingress_hook.yaml | 31 ++++++++ .../secret_hook.json | 71 ------------------- .../secret_hook.yaml | 31 ++++++++ test/integration-serial/serial_deploy_test.rb | 21 +++--- .../mutating_webhook_configuration_test.rb | 16 ++--- 8 files changed, 82 insertions(+), 164 deletions(-) delete mode 100644 test/fixtures/mutating_webhook_configurations/ingress_hook.json create mode 100644 test/fixtures/mutating_webhook_configurations/ingress_hook.yaml delete mode 100644 test/fixtures/mutating_webhook_configurations/secret_hook.json create mode 100644 test/fixtures/mutating_webhook_configurations/secret_hook.yaml diff --git a/lib/krane/cluster_resource_discovery.rb b/lib/krane/cluster_resource_discovery.rb index b2bb86d03..0a8c3a28d 100644 --- a/lib/krane/cluster_resource_discovery.rb +++ b/lib/krane/cluster_resource_discovery.rb @@ -66,7 +66,7 @@ def fetch_mutating_webhook_configurations raw_json, err, st = kubectl.run(*command, output: "json", attempts: 5, use_namespace: false) if st.success? JSON.parse(raw_json)["items"].map do |definition| - KubernetesResource::MutatingWebhookConfiguration.new(namespace: namespace, context: context, logger: logger, + Krane::MutatingWebhookConfiguration.new(namespace: namespace, context: context, logger: logger, definition: definition, statsd_tags: @namespace_tags) end else diff --git a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb index 02cf68b2e..d58b25074 100644 --- a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb +++ b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb @@ -2,10 +2,13 @@ module Krane class MutatingWebhookConfiguration < KubernetesResource + GLOBAL = true + class Webhook EQUIVALENT = 'Equivalent' EXACT = 'Exact' + class Rule def initialize(definition) @definition = definition diff --git a/test/fixtures/mutating_webhook_configurations/ingress_hook.json b/test/fixtures/mutating_webhook_configurations/ingress_hook.json deleted file mode 100644 index 874d497bb..000000000 --- a/test/fixtures/mutating_webhook_configurations/ingress_hook.json +++ /dev/null @@ -1,71 +0,0 @@ -{ - "apiVersion": "v1", - "items": [ - { - "apiVersion": "admissionregistration.k8s.io/v1", - "kind": "MutatingWebhookConfiguration", - "metadata": { - "creationTimestamp": "2020-08-04T19:49:44Z", - "generation": 2, - "name": "oauthboss-webhook-configuration", - "resourceVersion": "3115431", - "selfLink": "/apis/admissionregistration.k8s.io/v1/mutatingwebhookconfigurations/oauthboss-webhook-configuration", - "uid": "7afce875-5175-430d-95aa-de8ec97f15b0" - }, - "webhooks": [ - { - "admissionReviewVersions": [ - "v1beta1" - ], - "clientConfig": { - "caBundle": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUMwakNDQWJxZ0F3SUJBZ0lCQURBTkJna3Foa2lHOXcwQkFRc0ZBREFhTVJnd0ZnWURWUVFERXc5M1pXSm8KYjI5ckxXTmxjblF0WTJFd0hoY05NakF3T0RBME1UazBPVFExV2hjTk16QXdPREF5TVRrME9UUTFXakFhTVJndwpGZ1lEVlFRREV3OTNaV0pvYjI5ckxXTmxjblF0WTJFd2dnRWlNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUJEd0F3CmdnRUtBb0lCQVFDOGs1MjNobEFsald4RitITk8wWk9qQTZ4MGNSa1RQVHdkZm9nK3JGOXZ3Y21XRTBFT3kyYlUKZ3BNRU5SQzVpVzRxS1pCYUJ6MEU1bXcvbWNjcGNrbXR1V05pODVML0hzUjVlbWR2aWdUamJYb0RMeWxEdjNMLwpRSlVWOVRIYWFhaXh5cmFMNWhIRUw4bTBhTFNZcWc5MktxODJnWXB0aTB5azBnU1BqdjhUTHBFazhvdWp2L01YCnVJZWtQQzZOTWFDaU1BY1FBTVRyc1NiSmZqL0x0WGpwWk9FM2xCNWpTazNDUW1sT0t5Mm8zaDd6WTZ1RFNtN28KY3orMkhiUXhybCt1SkcrY1B4WVdoWXZReXpLWGViWU11ajd5Wm5FRmI2WFc3dC9ROVo2VmFzMi9DS1Z4Yll4LwppdXEvbU1xZk4xZnNvUitJTmVXVlJ3YlJ0dUJ5VkljRkFnTUJBQUdqSXpBaE1BNEdBMVVkRHdFQi93UUVBd0lDCnBEQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRQXc4VWQwOWRYR09kQkwKYnQ5M3JuYk41VzBkMkJUZk43OXJ6SVpadHlmMjREQVQ1SGc1QVV2Qmc4QjJNaFRqdU50VlN6dzN4bGV3TDk3aQo0Wk9JWGJPL3hZTWp3OXRibldDY2o0MlVKRnJUOEN2eVZEekExeS9VVE1lMFpyU3lOUGpWTlBaMmUrb3IvRFdkCmRtbmg0THZ6c3VNbVBPZk96NmUwZmljMzdtLzZjL3JZV0l6aTJYWWRwSllHa1J1eW51UlVUbDZpUjBOa0xXaGYKc25aNll6TnFXT2YvNi9xdHduNkhVckttYWN3L2Q3blBNSDRpQTFId1MyZFh6WkxVR3NVZkZIREFkU2FVSWR3cApsZDFUUjErQjVyMnhiUW95eVo4YnhuTk5Ud04rU2dFZGc5MjJKUFlaRkZFWldDL1NXOWVYcUZnTWM3RlQyMHh4ClE1NW1qdHRICi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K", - "service": { - "name": "oauthboss", - "namespace": "cloudbosses", - "path": "/oauthboss", - "port": 443 - } - }, - "failurePolicy": "Ignore", - "matchPolicy": "Exact", - "name": "oauthboss.cloudbosses.admission.online.shopify.com", - "namespaceSelector": { - "matchExpressions": [ - { - "key": "control-plane", - "operator": "DoesNotExist" - } - ] - }, - "objectSelector": {}, - "reinvocationPolicy": "Never", - "rules": [ - { - "apiGroups": [ - "extensions" - ], - "apiVersions": [ - "v1beta1" - ], - "operations": [ - "CREATE", - "UPDATE" - ], - "resources": [ - "ingresses" - ], - "scope": "*" - } - ], - "sideEffects": "Unknown", - "timeoutSeconds": 30 - } - ] - } - ], - "kind": "List", - "metadata": { - "resourceVersion": "", - "selfLink": "" - } -} diff --git a/test/fixtures/mutating_webhook_configurations/ingress_hook.yaml b/test/fixtures/mutating_webhook_configurations/ingress_hook.yaml new file mode 100644 index 000000000..a65ca650a --- /dev/null +++ b/test/fixtures/mutating_webhook_configurations/ingress_hook.yaml @@ -0,0 +1,31 @@ + +apiVersion: admissionregistration.k8s.io/v1beta1 +kind: MutatingWebhookConfiguration +metadata: + name: ingress-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1beta1 + clientConfig: + service: + name: ingress-hook + namespace: test + path: "/ingress-hook" + port: 443 + failurePolicy: Ignore + matchPolicy: Exact + name: ingress-hook.hooks.admission.krane.com + reinvocationPolicy: Never + rules: + - apiGroups: + - extensions + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + resources: + - ingresses + scope: "*" + sideEffects: Unknown + timeoutSeconds: 1 diff --git a/test/fixtures/mutating_webhook_configurations/secret_hook.json b/test/fixtures/mutating_webhook_configurations/secret_hook.json deleted file mode 100644 index 0c012c205..000000000 --- a/test/fixtures/mutating_webhook_configurations/secret_hook.json +++ /dev/null @@ -1,71 +0,0 @@ -{ - "apiVersion": "v1", - "items": [ - { - "apiVersion": "admissionregistration.k8s.io/v1", - "kind": "MutatingWebhookConfiguration", - "metadata": { - "creationTimestamp": "2020-08-04T19:49:44Z", - "generation": 2, - "name": "oauthboss-webhook-configuration", - "resourceVersion": "3115431", - "selfLink": "/apis/admissionregistration.k8s.io/v1/mutatingwebhookconfigurations/oauthboss-webhook-configuration", - "uid": "7afce875-5175-430d-95aa-de8ec97f15b0" - }, - "webhooks": [ - { - "admissionReviewVersions": [ - "v1beta1" - ], - "clientConfig": { - "caBundle": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUMwakNDQWJxZ0F3SUJBZ0lCQURBTkJna3Foa2lHOXcwQkFRc0ZBREFhTVJnd0ZnWURWUVFERXc5M1pXSm8KYjI5ckxXTmxjblF0WTJFd0hoY05NakF3T0RBME1UazBPVFExV2hjTk16QXdPREF5TVRrME9UUTFXakFhTVJndwpGZ1lEVlFRREV3OTNaV0pvYjI5ckxXTmxjblF0WTJFd2dnRWlNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUJEd0F3CmdnRUtBb0lCQVFDOGs1MjNobEFsald4RitITk8wWk9qQTZ4MGNSa1RQVHdkZm9nK3JGOXZ3Y21XRTBFT3kyYlUKZ3BNRU5SQzVpVzRxS1pCYUJ6MEU1bXcvbWNjcGNrbXR1V05pODVML0hzUjVlbWR2aWdUamJYb0RMeWxEdjNMLwpRSlVWOVRIYWFhaXh5cmFMNWhIRUw4bTBhTFNZcWc5MktxODJnWXB0aTB5azBnU1BqdjhUTHBFazhvdWp2L01YCnVJZWtQQzZOTWFDaU1BY1FBTVRyc1NiSmZqL0x0WGpwWk9FM2xCNWpTazNDUW1sT0t5Mm8zaDd6WTZ1RFNtN28KY3orMkhiUXhybCt1SkcrY1B4WVdoWXZReXpLWGViWU11ajd5Wm5FRmI2WFc3dC9ROVo2VmFzMi9DS1Z4Yll4LwppdXEvbU1xZk4xZnNvUitJTmVXVlJ3YlJ0dUJ5VkljRkFnTUJBQUdqSXpBaE1BNEdBMVVkRHdFQi93UUVBd0lDCnBEQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRQXc4VWQwOWRYR09kQkwKYnQ5M3JuYk41VzBkMkJUZk43OXJ6SVpadHlmMjREQVQ1SGc1QVV2Qmc4QjJNaFRqdU50VlN6dzN4bGV3TDk3aQo0Wk9JWGJPL3hZTWp3OXRibldDY2o0MlVKRnJUOEN2eVZEekExeS9VVE1lMFpyU3lOUGpWTlBaMmUrb3IvRFdkCmRtbmg0THZ6c3VNbVBPZk96NmUwZmljMzdtLzZjL3JZV0l6aTJYWWRwSllHa1J1eW51UlVUbDZpUjBOa0xXaGYKc25aNll6TnFXT2YvNi9xdHduNkhVckttYWN3L2Q3blBNSDRpQTFId1MyZFh6WkxVR3NVZkZIREFkU2FVSWR3cApsZDFUUjErQjVyMnhiUW95eVo4YnhuTk5Ud04rU2dFZGc5MjJKUFlaRkZFWldDL1NXOWVYcUZnTWM3RlQyMHh4ClE1NW1qdHRICi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K", - "service": { - "name": "oauthboss", - "namespace": "cloudbosses", - "path": "/oauthboss", - "port": 443 - } - }, - "failurePolicy": "Ignore", - "matchPolicy": "Equivalent", - "name": "oauthboss.cloudbosses.admission.online.shopify.com", - "namespaceSelector": { - "matchExpressions": [ - { - "key": "control-plane", - "operator": "DoesNotExist" - } - ] - }, - "objectSelector": {}, - "reinvocationPolicy": "Never", - "rules": [ - { - "apiGroups": [ - "core" - ], - "apiVersions": [ - "v1" - ], - "operations": [ - "CREATE", - "UPDATE" - ], - "resources": [ - "secrets" - ], - "scope": "*" - } - ], - "sideEffects": "Unknown", - "timeoutSeconds": 30 - } - ] - } - ], - "kind": "List", - "metadata": { - "resourceVersion": "", - "selfLink": "" - } -} diff --git a/test/fixtures/mutating_webhook_configurations/secret_hook.yaml b/test/fixtures/mutating_webhook_configurations/secret_hook.yaml new file mode 100644 index 000000000..f7603bd35 --- /dev/null +++ b/test/fixtures/mutating_webhook_configurations/secret_hook.yaml @@ -0,0 +1,31 @@ +--- +apiVersion: admissionregistration.k8s.io/v1beta1 +kind: MutatingWebhookConfiguration +metadata: + name: secretboss-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1beta1 + clientConfig: + service: + name: secretboss + namespace: cloudbosses + path: "/secretboss" + port: 443 + failurePolicy: Ignore + matchPolicy: Equivalent + name: secretboss.cloudbosses.admission.krane.com + reinvocationPolicy: Never + rules: + - apiGroups: + - core + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - secrets + scope: "*" + sideEffects: Unknown + timeoutSeconds: 1 diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index 49300fbbb..b26392c67 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -535,12 +535,14 @@ def test_batch_dry_run_apply_success_precludes_individual_resource_dry_run_valid end def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_side_dry_run - resp = mutating_webhook_fixture(File.join(fixture_path("mutating_webhook_configurations"), "ingress_hook.json")) - Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) + result = deploy_global_fixtures("mutating_webhook_configurations", subset: %(ingress_hook.yaml)) + assert_deploy_success(result) + Krane::ResourceDeployer.any_instance.expects(:dry_run).with do |params| - params.length == 3 && (params.map(&:type) - ["Deployment", "Service", "ConfigMap"]).empty? + # We expect the ingress field to not be called by the batch runner + params.length == 3 && (params.map(&:type).sort == ["ConfigMap", "Deployment", "Service"]) end - Krane::Ingress.any_instance.expects(:validate_definition) + Krane::Ingress.any_instance.expects(:validate_definition).with { |params| params[:dry_run] } result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb configmap-data.yml), render_erb: true) assert_deploy_success(result) assert_logs_match_all([ @@ -550,11 +552,12 @@ def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_sid end def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency_does_not_fail_batch_running - resp = mutating_webhook_fixture(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.json")) - Krane::ClusterResourceDiscovery.any_instance.expects(:fetch_mutating_webhook_configurations).returns(resp) - expected_dry_runs = 0 + result = deploy_global_fixtures("mutating_webhook_configurations", subset: %(secret_hook.yaml)) + assert_deploy_success(result) + + actual_dry_runs = 0 Krane::KubernetesResource.any_instance.expects(:validate_definition).with do |params| - expected_dry_runs += 1 if params[:dry_run] + actual_dry_runs += 1 if params[:dry_run] true end.times(5) result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb secret.yml configmap-data.yml), @@ -568,7 +571,7 @@ def test_resources_with_side_effect_inducing_webhooks_with_transitive_dependency }] end assert_deploy_success(result) - assert_equal(expected_dry_runs, 1) + assert_equal(actual_dry_runs, 1) assert_logs_match_all([ "Result: SUCCESS", "Successfully deployed 5 resources", diff --git a/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb b/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb index 927cdfdbf..08237af97 100644 --- a/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb +++ b/test/unit/krane/kubernetes_resource/mutating_webhook_configuration_test.rb @@ -3,9 +3,7 @@ class MutatingWebhookConfigurationTest < Krane::TestCase def test_load_from_json - definition = JSON.parse( - File.read(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.json")) - )["items"][0] + definition = YAML.load_file(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.yaml")) webhook_configuration = Krane::MutatingWebhookConfiguration.new( namespace: 'test', context: 'nope', definition: definition, logger: @logger, statsd_tags: nil @@ -35,9 +33,7 @@ def test_webhook_configuration_matches_when_side_effects secret = Krane::Secret.new(namespace: 'test', context: 'nope', definition: secret_def, logger: @logger, statsd_tags: nil) - definition = JSON.parse( - File.read(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.json")) - )["items"][0] + definition = YAML.load_file(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.yaml")) webhook_configuration = Krane::MutatingWebhookConfiguration.new( namespace: 'test', context: 'nope', definition: definition, logger: @logger, statsd_tags: nil @@ -54,9 +50,7 @@ def test_matches_webhook_configuration_doesnt_match_when_no_side_effects_and_fla secret = Krane::Secret.new(namespace: 'test', context: 'nope', definition: secret_def, logger: @logger, statsd_tags: nil) - definition = JSON.parse( - File.read(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.json")) - )["items"][0] + definition = YAML.load_file(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.yaml")) webhook_configuration = Krane::MutatingWebhookConfiguration.new( namespace: 'test', context: 'nope', definition: definition, logger: @logger, statsd_tags: nil @@ -73,9 +67,7 @@ def test_no_match_when_policy_is_exact_and_resource_doesnt_match secret = Krane::Secret.new(namespace: 'test', context: 'nope', definition: secret_def, logger: @logger, statsd_tags: nil) - definition = JSON.parse( - File.read(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.json")) - )["items"][0] + definition = YAML.load_file(File.join(fixture_path("mutating_webhook_configurations"), "secret_hook.yaml")) webhook_configuration = Krane::MutatingWebhookConfiguration.new( namespace: 'test', context: 'nope', definition: definition, logger: @logger, statsd_tags: nil From 2225ceabeb11847fc3ae2fe2e725ef86fb979177 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Mon, 8 Feb 2021 10:44:00 -0500 Subject: [PATCH 23/27] cop --- lib/krane/kubernetes_resource/mutating_webhook_configuration.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb index d58b25074..e1cbf5ace 100644 --- a/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb +++ b/lib/krane/kubernetes_resource/mutating_webhook_configuration.rb @@ -8,7 +8,6 @@ class Webhook EQUIVALENT = 'Equivalent' EXACT = 'Exact' - class Rule def initialize(definition) @definition = definition From 86370b251aee43a9c5ce77ca73b11d2345db305a Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Mon, 8 Feb 2021 11:08:26 -0500 Subject: [PATCH 24/27] more test fix --- test/integration-serial/serial_deploy_test.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index b26392c67..8246aa817 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -539,8 +539,11 @@ def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_sid assert_deploy_success(result) Krane::ResourceDeployer.any_instance.expects(:dry_run).with do |params| - # We expect the ingress field to not be called by the batch runner params.length == 3 && (params.map(&:type).sort == ["ConfigMap", "Deployment", "Service"]) + end.returns(true) + + [Krane::ConfigMap, Krane::Deployment, Krane::Service].each do |r| + r.any_instance.expects(:validate_definition).with { |params| params[:dry_run] == false } end Krane::Ingress.any_instance.expects(:validate_definition).with { |params| params[:dry_run] } result = deploy_fixtures('hello-cloud', subset: %w(web.yml.erb configmap-data.yml), render_erb: true) From 7e208307bc6f88c46b78e0f56a9658f3e3e4ab67 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Mon, 8 Feb 2021 11:09:43 -0500 Subject: [PATCH 25/27] comment --- test/integration-serial/serial_deploy_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/integration-serial/serial_deploy_test.rb b/test/integration-serial/serial_deploy_test.rb index 8246aa817..5ece0d223 100644 --- a/test/integration-serial/serial_deploy_test.rb +++ b/test/integration-serial/serial_deploy_test.rb @@ -539,6 +539,7 @@ def test_resources_with_side_effect_inducing_webhooks_are_not_batched_server_sid assert_deploy_success(result) Krane::ResourceDeployer.any_instance.expects(:dry_run).with do |params| + # We expect the ingress to not be included in the batch run params.length == 3 && (params.map(&:type).sort == ["ConfigMap", "Deployment", "Service"]) end.returns(true) From dd154533f583106f5b48b5c8b95f41efad5a60c1 Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Mon, 8 Feb 2021 11:15:37 -0500 Subject: [PATCH 26/27] cleanup fixtures --- .../mutating_webhook_configurations/ingress_hook.yaml | 2 +- .../mutating_webhook_configurations/secret_hook.yaml | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/fixtures/mutating_webhook_configurations/ingress_hook.yaml b/test/fixtures/mutating_webhook_configurations/ingress_hook.yaml index a65ca650a..55e0c4c7a 100644 --- a/test/fixtures/mutating_webhook_configurations/ingress_hook.yaml +++ b/test/fixtures/mutating_webhook_configurations/ingress_hook.yaml @@ -1,4 +1,4 @@ - +--- apiVersion: admissionregistration.k8s.io/v1beta1 kind: MutatingWebhookConfiguration metadata: diff --git a/test/fixtures/mutating_webhook_configurations/secret_hook.yaml b/test/fixtures/mutating_webhook_configurations/secret_hook.yaml index f7603bd35..9f9689007 100644 --- a/test/fixtures/mutating_webhook_configurations/secret_hook.yaml +++ b/test/fixtures/mutating_webhook_configurations/secret_hook.yaml @@ -2,19 +2,19 @@ apiVersion: admissionregistration.k8s.io/v1beta1 kind: MutatingWebhookConfiguration metadata: - name: secretboss-webhook-configuration + name: secret-hook-webhook-configuration webhooks: - admissionReviewVersions: - v1beta1 clientConfig: service: - name: secretboss - namespace: cloudbosses - path: "/secretboss" + name: secret-hook + namespace: test + path: "/secret-hook" port: 443 failurePolicy: Ignore matchPolicy: Equivalent - name: secretboss.cloudbosses.admission.krane.com + name: secret-hook.hooks.admission.krane.com reinvocationPolicy: Never rules: - apiGroups: From c0677d2702722546a4747ec1ad199e1a2025091d Mon Sep 17 00:00:00 2001 From: Timothy Smith Date: Tue, 9 Feb 2021 09:17:50 -0500 Subject: [PATCH 27/27] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a59fcb26..77452fc8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## next +- Remove resources that are targeted by side-effect-inducing mutating admission webhooks from the serverside dry run batch [#798](https://github.com/Shopify/krane/pull/798) + ## 2.1.5 - Fix bug where the wrong dry-run flag is used for kubectl if client version is below 1.18 AND server version is 1.18+ [#793](https://github.com/Shopify/krane/pull/793).