Merge pull request #738 from Shopify/remove-kubernetes-deploy-annotat…

…ion-prefix

Remove kubernetes deploy annotation prefix

lib/krane/annotation.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module Krane
+  module Annotation
+    class << self
+      def for(suffix)
+        "krane.shopify.io/#{suffix}"
+      end
+    end
+  end
+end

lib/krane/concerns/template_reporting.rb

@@ -15,12 +15,6 @@ def record_invalid_template(logger:, err:, filename:, content: nil)
       logger.summary.add_paragraph(debug_msg)
     end
 
-    def record_warnings(logger:, warning:, filename:)
-      warn_msg = "Template warning: #{filename}\n"
-      warn_msg += "> Warning message:\n#{Krane::FormattedLogger.indent_four(warning)}"
-      logger.summary.add_paragraph(ColorizedString.new(warn_msg).yellow)
-    end
-
     def add_para_from_list(logger:, action:, enum:)
       logger.summary.add_action(action)
       logger.summary.add_paragraph(enum.map { |e| "- #{e}" }.join("\n"))

lib/krane/deploy_task.rb

@@ -4,6 +4,7 @@
 require 'tempfile'
 require 'fileutils'
 
+require 'krane/annotation'
 require 'krane/common'
 require 'krane/concurrency'
 require 'krane/resource_cache'
@@ -282,11 +283,6 @@ def validate_resources(resources)
         r.validate_definition(kubectl, selector: @selector)
       end
 
-      resources.select(&:has_warnings?).each do |resource|
-        record_warnings(logger: @logger, warning: resource.validation_warning_msg,
-          filename: File.basename(resource.file_path))
-      end
-
       failed_resources = resources.select(&:validation_failed?)
       if failed_resources.present?
 

lib/krane/ejson_secret_provisioner.rb

@@ -12,10 +12,10 @@ def initialize(msg)
   end
 
   class EjsonSecretProvisioner
-    EJSON_SECRET_ANNOTATION = "kubernetes-deploy.shopify.io/ejson-secret"
     EJSON_SECRET_KEY = "kubernetes_secrets"
     EJSON_SECRETS_FILE = "secrets.ejson"
     EJSON_KEYS_SECRET = "ejson-keys"
+
     delegate :namespace, :context, :logger, to: :@task_config
 
     def initialize(task_config:, ejson_keys_secret:, ejson_file:, statsd_tags:, selector: nil)
@@ -106,7 +106,6 @@ def generate_secret_resource(secret_name, secret_type, data)
           "name" => secret_name,
           "labels" => labels,
           "namespace" => namespace,
-          "annotations" => { EJSON_SECRET_ANNOTATION => "true" },
         },
         "data" => encoded_data,
       }

lib/krane/global_deploy_task.rb

@@ -133,11 +133,6 @@ def validate_resources(resources)
         r.validate_definition(@kubectl, selector: @selector)
       end
 
-      resources.select(&:has_warnings?).each do |resource|
-        record_warnings(logger: logger, warning: resource.validation_warning_msg,
-          filename: File.basename(resource.file_path))
-      end
-
       failed_resources = resources.select(&:validation_failed?)
       if failed_resources.present?
         failed_resources.each do |r|

lib/krane/kubernetes_resource.rb

@@ -32,9 +32,7 @@ class KubernetesResource
       If you have reason to believe it will succeed, retry the deploy to continue to monitor the rollout.
       MSG
 
-    TIMEOUT_OVERRIDE_ANNOTATION_SUFFIX = "timeout-override"
-    TIMEOUT_OVERRIDE_ANNOTATION_DEPRECATED = "kubernetes-deploy.shopify.io/#{TIMEOUT_OVERRIDE_ANNOTATION_SUFFIX}"
-    TIMEOUT_OVERRIDE_ANNOTATION = "krane.shopify.io/#{TIMEOUT_OVERRIDE_ANNOTATION_SUFFIX}"
+    TIMEOUT_OVERRIDE_ANNOTATION = "timeout-override"
     LAST_APPLIED_ANNOTATION = "kubectl.kubernetes.io/last-applied-configuration"
     SENSITIVE_TEMPLATE_CONTENT = false
     SERVER_DRY_RUNNABLE = false
@@ -103,7 +101,7 @@ def timeout
     def timeout_override
       return @timeout_override if defined?(@timeout_override)
 
-      @timeout_override = DurationParser.new(krane_annotation_value(TIMEOUT_OVERRIDE_ANNOTATION_SUFFIX)).parse!.to_i
+      @timeout_override = DurationParser.new(krane_annotation_value(TIMEOUT_OVERRIDE_ANNOTATION)).parse!.to_i
     rescue DurationParser::ParsingError
       @timeout_override = nil
     end
@@ -123,7 +121,6 @@ def initialize(namespace:, context:, definition:, logger:, statsd_tags: [])
       @statsd_report_done = false
       @disappeared = false
       @validation_errors = []
-      @validation_warnings = []
       @instance_data = {}
       @server_dry_run_validated = false
     end
@@ -134,22 +131,12 @@ def to_kubeclient_resource
 
     def validate_definition(kubectl, selector: nil)
       @validation_errors = []
-      @validation_warnings = []
       validate_selector(selector) if selector
       validate_timeout_annotation
-      validate_annotation_version
       validate_spec_with_kubectl(kubectl)
       @validation_errors.present?
     end
 
-    def validation_warning_msg
-      @validation_warnings.join("\n")
-    end
-
-    def has_warnings?
-      @validation_warnings.present?
-    end
-
     def validation_error_msg
       @validation_errors.join("\n")
     end
@@ -500,8 +487,8 @@ def global?
     private
 
     def validate_timeout_annotation
-      timeout_override_value = krane_annotation_value(TIMEOUT_OVERRIDE_ANNOTATION_SUFFIX)
-      timeout_annotation_key = krane_annotation_key(TIMEOUT_OVERRIDE_ANNOTATION_SUFFIX)
+      timeout_override_value = krane_annotation_value(TIMEOUT_OVERRIDE_ANNOTATION)
+      timeout_annotation_key = Annotation.for(TIMEOUT_OVERRIDE_ANNOTATION)
       return if timeout_override_value.nil?
 
       override = DurationParser.new(timeout_override_value).parse!
@@ -514,30 +501,8 @@ def validate_timeout_annotation
       @validation_errors << "#{timeout_annotation_key} annotation is invalid: #{e}"
     end
 
-    def validate_annotation_version
-      return if validation_warning_msg.include?("annotations is deprecated")
-      annotation_keys = @definition.dig("metadata", "annotations")&.keys
-      annotation_keys&.each do |annotation|
-        if annotation.include?("kubernetes-deploy.shopify.io")
-          annotation_prefix = annotation.split('/').first
-          @validation_warnings << "#{annotation_prefix} as a prefix for annotations is deprecated: "\
-            "Use the 'krane.shopify.io' annotation prefix instead"
-          return
-        end
-      end
-    end
-
     def krane_annotation_value(suffix)
-      @definition.dig("metadata", "annotations", "kubernetes-deploy.shopify.io/#{suffix}") ||
-        @definition.dig("metadata", "annotations", "krane.shopify.io/#{suffix}")
-    end
-
-    def krane_annotation_key(suffix)
-      if @definition.dig("metadata", "annotations", "kubernetes-deploy.shopify.io/#{suffix}")
-        "kubernetes-deploy.shopify.io/#{suffix}"
-      elsif @definition.dig("metadata", "annotations", "krane.shopify.io/#{suffix}")
-        "krane.shopify.io/#{suffix}"
-      end
+      @definition.dig("metadata", "annotations", Annotation.for(suffix))
     end
 
     def validate_selector(selector)

lib/krane/kubernetes_resource/custom_resource.rb

@@ -70,7 +70,7 @@ def validate_definition(*, **)
       @validation_errors << "The CRD that specifies this resource is using invalid rollout conditions. " \
       "Krane will not be able to continue until those rollout conditions are fixed.\n" \
       "Rollout conditions can be found on the CRD that defines this resource (#{@crd.name}), " \
-      "under the annotation #{CustomResourceDefinition::ROLLOUT_CONDITIONS_ANNOTATION}.\n" \
+      "under the annotation #{Annotation.for(CustomResourceDefinition::ROLLOUT_CONDITIONS_ANNOTATION)}.\n" \
       "Validation failed with: #{e}"
     end
 

lib/krane/kubernetes_resource/custom_resource_definition.rb

@@ -2,9 +2,8 @@
 module Krane
   class CustomResourceDefinition < KubernetesResource
     TIMEOUT = 2.minutes
-    ROLLOUT_CONDITIONS_ANNOTATION_SUFFIX = "instance-rollout-conditions"
-    ROLLOUT_CONDITIONS_ANNOTATION = "krane.shopify.io/#{ROLLOUT_CONDITIONS_ANNOTATION_SUFFIX}"
-    TIMEOUT_FOR_INSTANCE_ANNOTATION = "krane.shopify.io/instance-timeout"
+    ROLLOUT_CONDITIONS_ANNOTATION = "instance-rollout-conditions"
+    TIMEOUT_FOR_INSTANCE_ANNOTATION = "instance-timeout"
     GLOBAL = true
 
     def deploy_succeeded?
@@ -20,7 +19,7 @@ def timeout_message
     end
 
     def timeout_for_instance
-      timeout = krane_annotation_value("instance-timeout")
+      timeout = krane_annotation_value(TIMEOUT_FOR_INSTANCE_ANNOTATION)
       DurationParser.new(timeout).parse!.to_i
     rescue DurationParser::ParsingError
       nil
@@ -63,8 +62,8 @@ def predeployed?
     def rollout_conditions
       return @rollout_conditions if defined?(@rollout_conditions)
 
-      @rollout_conditions = if krane_annotation_value(ROLLOUT_CONDITIONS_ANNOTATION_SUFFIX)
-        RolloutConditions.from_annotation(krane_annotation_value(ROLLOUT_CONDITIONS_ANNOTATION_SUFFIX))
+      @rollout_conditions = if krane_annotation_value(ROLLOUT_CONDITIONS_ANNOTATION)
+        RolloutConditions.from_annotation(krane_annotation_value(ROLLOUT_CONDITIONS_ANNOTATION))
       end
     rescue RolloutConditionsError
       @rollout_conditions = nil
@@ -75,13 +74,13 @@ def validate_definition(*, **)
 
       validate_rollout_conditions
     rescue RolloutConditionsError => e
-      @validation_errors << "Annotation #{krane_annotation_key(ROLLOUT_CONDITIONS_ANNOTATION_SUFFIX)} "\
+      @validation_errors << "Annotation #{Annotation.for(ROLLOUT_CONDITIONS_ANNOTATION)} " \
         "on #{name} is invalid: #{e}"
     end
 
     def validate_rollout_conditions
-      if krane_annotation_value(ROLLOUT_CONDITIONS_ANNOTATION_SUFFIX) && @rollout_conditions_validated.nil?
-        conditions = RolloutConditions.from_annotation(krane_annotation_value(ROLLOUT_CONDITIONS_ANNOTATION_SUFFIX))
+      if krane_annotation_value(ROLLOUT_CONDITIONS_ANNOTATION) && @rollout_conditions_validated.nil?
+        conditions = RolloutConditions.from_annotation(krane_annotation_value(ROLLOUT_CONDITIONS_ANNOTATION))
         conditions.validate!
       end
 

lib/krane/kubernetes_resource/deployment.rb

@@ -5,9 +5,7 @@ module Krane
   class Deployment < KubernetesResource
     TIMEOUT = 7.minutes
     SYNC_DEPENDENCIES = %w(Pod ReplicaSet)
-    REQUIRED_ROLLOUT_ANNOTATION_SUFFIX = "required-rollout"
-    REQUIRED_ROLLOUT_ANNOTATION_DEPRECATED = "kubernetes-deploy.shopify.io/#{REQUIRED_ROLLOUT_ANNOTATION_SUFFIX}"
-    REQUIRED_ROLLOUT_ANNOTATION = "krane.shopify.io/#{REQUIRED_ROLLOUT_ANNOTATION_SUFFIX}"
+    REQUIRED_ROLLOUT_ANNOTATION = "required-rollout"
     REQUIRED_ROLLOUT_TYPES = %w(maxUnavailable full none).freeze
     DEFAULT_REQUIRED_ROLLOUT = 'full'
 
@@ -106,7 +104,7 @@ def validate_definition(*, **)
 
       strategy = @definition.dig('spec', 'strategy', 'type').to_s
       if required_rollout.downcase == 'maxunavailable' && strategy.present? && strategy.downcase != 'rollingupdate'
-        @validation_errors << "'#{krane_annotation_key(REQUIRED_ROLLOUT_ANNOTATION_SUFFIX)}: #{required_rollout}' "\
+        @validation_errors << "'#{Annotation.for(REQUIRED_ROLLOUT_ANNOTATION)}: #{required_rollout}' " \
           "is incompatible with strategy '#{strategy}'"
       end
 
@@ -151,7 +149,7 @@ def progress_deadline
     end
 
     def rollout_annotation_err_msg
-      "'#{krane_annotation_key(REQUIRED_ROLLOUT_ANNOTATION_SUFFIX)}: #{required_rollout}' is invalid. "\
+      "'#{Annotation.for(REQUIRED_ROLLOUT_ANNOTATION)}: #{required_rollout}' is invalid. " \
         "Acceptable values: #{REQUIRED_ROLLOUT_TYPES.join(', ')}"
     end
 

test/helpers/fixture_set.rb

@@ -130,10 +130,9 @@ def assert_pod_templates_present(cm_name)
       assert_equal(1, pod_templates.size, "Expected 1 podtemplate, got #{pod_templates.size}")
     end
 
-    def assert_secret_present(secret_name, expected_data = nil, type: 'Opaque', ejson: false)
+    def assert_secret_present(secret_name, expected_data = nil, type: 'Opaque')
       secret = kubeclient.get_secret(secret_name, namespace)
       refute_nil(secret, "Secret `#{secret_name}` not found")
-      assert_annotated(secret, Krane::EjsonSecretProvisioner::EJSON_SECRET_ANNOTATION) if ejson
       assert_equal(type, secret["type"])
       return unless expected_data
 

test/helpers/fixture_sets/ejson_cloud.rb

@@ -37,12 +37,12 @@ def catphotoscom_key_value
     def assert_all_secrets_present
       assert_secret_present("ejson-keys")
       cert_data = { "tls.crt" => "this-is-the-certificate", "tls.key" => catphotoscom_key_value }
-      assert_secret_present("catphotoscom", cert_data, type: "kubernetes.io/tls", ejson: true)
+      assert_secret_present("catphotoscom", cert_data, type: "kubernetes.io/tls")
 
       token_data = { "api-token" => "this-is-the-api-token", "service" => "Datadog" } # Impt: it isn't _service: Datadog
-      assert_secret_present("monitoring-token", token_data, ejson: true)
+      assert_secret_present("monitoring-token", token_data)
 
-      assert_secret_present("unused-secret", { "this-is-for-testing-deletion" => "true" }, ejson: true)
+      assert_secret_present("unused-secret", "this-is-for-testing-deletion" => "true")
     end
 
     def assert_web_resources_up