Skip to content

Commit

Permalink
Add warning on depricated annotation
Browse files Browse the repository at this point in the history
  • Loading branch information
Ryan Brushett committed Sep 6, 2019
1 parent 968e74c commit f643581
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 0 deletions.
12 changes: 12 additions & 0 deletions lib/kubernetes-deploy/deploy_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,12 @@ def validate_resources(resources)
KubernetesDeploy::Concurrency.split_across_threads(resources) do |r|
r.validate_definition(kubectl, selector: @selector)
end

warned_resources = resources.select(&:has_warnings?)
warned_resources.each do |r|
record_warnings(warning: r.validation_warning_msg, filename: File.basename(r.file_path))
end

failed_resources = resources.select(&:validation_failed?)
return unless failed_resources.present?

Expand Down Expand Up @@ -319,6 +325,12 @@ def record_invalid_template(err:, filename:, content: nil)
@logger.summary.add_paragraph(debug_msg)
end

def record_warnings(warning:, filename:)
warn_msg = "Template warning: #{filename}\n"
warn_msg += "> Warning message:\n#{FormattedLogger.indent_four(warning)}"
@logger.summary.add_paragraph(ColorizedString.new(warn_msg).yellow)
end

def validate_configuration(allow_protected_ns:, prune:)
errors = []
errors += kubeclient_builder.validate_config_files
Expand Down
20 changes: 20 additions & 0 deletions lib/kubernetes-deploy/kubernetes_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ def initialize(namespace:, context:, definition:, logger:, statsd_tags: [])
@statsd_report_done = false
@disappeared = false
@validation_errors = []
@validation_warnings = []
@instance_data = {}
end

Expand All @@ -124,12 +125,22 @@ 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
Expand Down Expand Up @@ -410,6 +421,15 @@ def validate_timeout_annotation
@validation_errors << "#{annotation_prefix} annotation is invalid: #{e}"
end

def validate_annotation_version
annotation_keys = @definition.dig("metadata", "annotations")&.keys
annotation_keys&.any? do |annotation|
if annotation.include?("kubernetes-deploy.shopify.io")
@validation_warnings << "#{annotation} is deprecated: Use 'krane.shopify.io' annotations instead"
end
end
end

def timeout_annotation
@definition.dig("metadata", "annotations", TIMEOUT_OVERRIDE_ANNOTATION_DEPRECATED) ||
@definition.dig("metadata", "annotations", TIMEOUT_OVERRIDE_ANNOTATION)
Expand Down
15 changes: 15 additions & 0 deletions test/unit/kubernetes-deploy/kubernetes_resource_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,21 @@ def test_fetch_events_returns_empty_hash_when_kubectl_results_empty
assert_operator(events, :empty?)
end

def test_deprecated_annotation_generates_warning
customized_resource = DummyResource.new(definition_extras: build_timeout_metadata_deprecated("60S"))
customized_resource.validate_definition(kubectl)
assert(customized_resource.has_warnings?, "Deprecated annotation did not generate a warning")
assert_equal("kubernetes-deploy.shopify.io/timeout-override is deprecated: "\
"Use 'krane.shopify.io' annotations instead",
customized_resource.validation_warning_msg)
end

def test_valid_annotations_generate_no_warning
customized_resource = DummyResource.new(definition_extras: build_timeout_metadata("60S"))
customized_resource.validate_definition(kubectl)
refute(customized_resource.has_warnings?, "Valid annotation generated a warning")
end

def test_can_override_hardcoded_timeout_via_an_annotation_deprecated
basic_resource = DummyResource.new
assert_equal(300, basic_resource.timeout)
Expand Down

0 comments on commit f643581

Please sign in to comment.