Skip to content

Commit

Permalink
allow specifying additional/default labels via command line
Browse files Browse the repository at this point in the history
This change adds the ability to set additional labels or provide
default values for deployment commands. This also works for ejson
secrets. `ejson-keys` as shared secret will not be labled.

The functionality was not made available on `krane render` due to
potentially confusing behaviour around labels on secrets when
using `krane render … | krane deploy -f secrets.ejson -f -` .

Allowing labels specified in the templates take precedence is an
intentional choice. It is the more flexible approach and allows
customization for edge cases like migrations and "nested"
deployments.

see #682
  • Loading branch information
breunigs committed Nov 28, 2023
1 parent b87c0c0 commit e848750
Show file tree
Hide file tree
Showing 15 changed files with 164 additions and 16 deletions.
6 changes: 6 additions & 0 deletions lib/krane/cli/deploy_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ class DeployCommand
desc: "Use --selector as a label filter to deploy only a subset "\
"of the provided resources",
default: false },
"extra-labels" => { type: :string, banner: "'label=value,foo=bar'",
desc: "Labels to set on resources that don't have them" },
"verbose-log-prefix" => { type: :boolean, desc: "Add [context][namespace] to the log prefix",
default: false },
"verify-result" => { type: :boolean, default: true,
Expand All @@ -39,6 +41,7 @@ def self.from_options(namespace, context, options)
require 'krane/deploy_task'
require 'krane/options_helper'
require 'krane/label_selector'
require 'krane/extra_labels'

selector = ::Krane::LabelSelector.parse(options[:selector]) if options[:selector]
selector_as_filter = options['selector-as-filter']
Expand All @@ -47,6 +50,8 @@ def self.from_options(namespace, context, options)
raise(Thor::RequiredArgumentMissingError, '--selector must be set when --selector-as-filter is set')
end

extra_labels = ::Krane::ExtraLabels.parse(options['extra-labels']) if options['extra-labels']

logger = ::Krane::FormattedLogger.build(namespace, context,
verbose_prefix: options['verbose-log-prefix'])

Expand All @@ -71,6 +76,7 @@ def self.from_options(namespace, context, options)
selector: selector,
selector_as_filter: selector_as_filter,
protected_namespaces: protected_namespaces,
extra_labels: extra_labels,
)

deploy.run!(
Expand Down
6 changes: 6 additions & 0 deletions lib/krane/cli/global_deploy_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ class GlobalDeployCommand
desc: "Use --selector as a label filter to deploy only a subset "\
"of the provided resources",
default: false },
"extra-labels" => { type: :string, banner: "'label=value,foo=bar'",
desc: "Labels to set on resources that don't have them" },
"prune" => { type: :boolean, desc: "Enable deletion of resources that match"\
" the provided selector and do not appear in the provided templates",
default: true },
Expand All @@ -29,6 +31,7 @@ def self.from_options(context, options)
require 'krane/global_deploy_task'
require 'krane/options_helper'
require 'krane/label_selector'
require 'krane/extra_labels'
require 'krane/duration_parser'

selector = ::Krane::LabelSelector.parse(options[:selector])
Expand All @@ -38,6 +41,8 @@ def self.from_options(context, options)
raise(Thor::RequiredArgumentMissingError, '--selector must be set when --selector-as-filter is set')
end

extra_labels = ::Krane::ExtraLabels.parse(options['extra-labels']) if options['extra-labels']

filenames = options[:filenames].dup
filenames << "-" if options[:stdin]
if filenames.empty?
Expand All @@ -51,6 +56,7 @@ def self.from_options(context, options)
global_timeout: ::Krane::DurationParser.new(options["global-timeout"]).parse!.to_i,
selector: selector,
selector_as_filter: selector_as_filter,
extra_labels: extra_labels,
)

deploy.run!(
Expand Down
7 changes: 5 additions & 2 deletions lib/krane/deploy_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,13 @@ def server_version
# @param global_timeout [Integer] Timeout in seconds
# @param selector [Hash] Selector(s) parsed by Krane::LabelSelector
# @param selector_as_filter [Boolean] Allow selecting a subset of Kubernetes resource templates to deploy
# @param extra_labels [Hash] labels to set on resources that don't have them
# @param filenames [Array<String>] An array of filenames and/or directories containing templates (*required*)
# @param protected_namespaces [Array<String>] Array of protected Kubernetes namespaces (defaults
# to Krane::DeployTask::PROTECTED_NAMESPACES)
# @param render_erb [Boolean] Enable ERB rendering
def initialize(namespace:, context:, current_sha: nil, logger: nil, kubectl_instance: nil, bindings: {},
global_timeout: nil, selector: nil, selector_as_filter: false, filenames: [], protected_namespaces: nil,
global_timeout: nil, selector: nil, selector_as_filter: false, extra_labels: {}, filenames: [], protected_namespaces: nil,
render_erb: false, kubeconfig: nil)
@logger = logger || Krane::FormattedLogger.build(namespace, context)
@template_sets = TemplateSets.from_dirs_and_files(paths: filenames, logger: @logger, render_erb: render_erb)
Expand All @@ -119,6 +120,7 @@ def initialize(namespace:, context:, current_sha: nil, logger: nil, kubectl_inst
@global_timeout = global_timeout
@selector = selector
@selector_as_filter = selector_as_filter
@extra_labels = extra_labels
@protected_namespaces = protected_namespaces || PROTECTED_NAMESPACES
@render_erb = render_erb
end
Expand Down Expand Up @@ -211,6 +213,7 @@ def ejson_provisioners
ejson_file: ejson_secret_file,
statsd_tags: @namespace_tags,
selector: @selector,
extra_labels: @extra_labels,
)
end
end
Expand Down Expand Up @@ -241,7 +244,7 @@ def discover_resources
@template_sets.with_resource_definitions(current_sha: @current_sha, bindings: @bindings) do |r_def|
crd = crds_by_kind[r_def["kind"]]&.first
r = KubernetesResource.build(namespace: @namespace, context: @context, logger: @logger, definition: r_def,
statsd_tags: @namespace_tags, crd: crd, global_names: @task_config.global_kinds)
statsd_tags: @namespace_tags, crd: crd, global_names: @task_config.global_kinds, extra_labels: @extra_labels)
resources << r
@logger.info(" - #{r.id}")
end
Expand Down
4 changes: 3 additions & 1 deletion lib/krane/ejson_secret_provisioner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ class EjsonSecretProvisioner

delegate :namespace, :context, :logger, to: :@task_config

def initialize(task_config:, ejson_keys_secret:, ejson_file:, statsd_tags:, selector: nil)
def initialize(task_config:, ejson_keys_secret:, ejson_file:, statsd_tags:, selector: nil, extra_labels: {})
@ejson_keys_secret = ejson_keys_secret
@ejson_file = ejson_file
@statsd_tags = statsd_tags
@selector = selector
@extra_labels = extra_labels
@task_config = task_config
@kubectl = Kubectl.new(
task_config: @task_config,
Expand Down Expand Up @@ -97,6 +98,7 @@ def generate_secret_resource(secret_name, secret_type, data)

labels = { "name" => secret_name }
labels.reverse_merge!(@selector.to_h) if @selector
labels.reverse_merge!(@extra_labels)

secret = {
'kind' => 'Secret',
Expand Down
21 changes: 21 additions & 0 deletions lib/krane/extra_labels.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# frozen_string_literal: true

module Krane
class ExtraLabels
def self.parse(string)
extra_labels = {}

string.split(',').each do |kvp|
key, value = kvp.split('=', 2)

if key.blank?
raise ArgumentError, "key is blank"
end

extra_labels[key] = value
end

extra_labels
end
end
end
6 changes: 4 additions & 2 deletions lib/krane/global_deploy_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ class GlobalDeployTask
# @param global_timeout [Integer] Timeout in seconds
# @param selector [Hash] Selector(s) parsed by Krane::LabelSelector (*required*)
# @param selector_as_filter [Boolean] Allow selecting a subset of Kubernetes resource templates to deploy
# @param extra_labels [Hash] labels to set on resources that don't have them
# @param filenames [Array<String>] An array of filenames and/or directories containing templates (*required*)
def initialize(context:, global_timeout: nil, selector: nil, selector_as_filter: false,
filenames: [], logger: nil, kubeconfig: nil)
extra_labels: {}, filenames: [], logger: nil, kubeconfig: nil)
template_paths = filenames.map { |path| File.expand_path(path) }

@task_config = TaskConfig.new(context, nil, logger, kubeconfig)
Expand All @@ -45,6 +46,7 @@ def initialize(context:, global_timeout: nil, selector: nil, selector_as_filter:
@global_timeout = global_timeout
@selector = selector
@selector_as_filter = selector_as_filter
@extra_labels = extra_labels
end

# Runs the task, returning a boolean representing success or failure
Expand Down Expand Up @@ -168,7 +170,7 @@ def discover_resources
@template_sets.with_resource_definitions do |r_def|
crd = crds_by_kind[r_def["kind"]]&.first
r = KubernetesResource.build(context: context, logger: logger, definition: r_def,
crd: crd, global_names: global_kinds, statsd_tags: statsd_tags)
crd: crd, global_names: global_kinds, statsd_tags: statsd_tags, extra_labels: @extra_labels)
resources << r
logger.info(" - #{r.id}")
end
Expand Down
16 changes: 12 additions & 4 deletions lib/krane/kubernetes_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ class KubernetesResource
SYNC_DEPENDENCIES = []

class << self
def build(namespace: nil, context:, definition:, logger:, statsd_tags:, crd: nil, global_names: [])
def build(namespace: nil, context:, definition:, logger:, statsd_tags:, crd: nil, global_names: [], extra_labels: {})
validate_definition_essentials(definition)
opts = { namespace: namespace, context: context, definition: definition, logger: logger,
statsd_tags: statsd_tags }
statsd_tags: statsd_tags, extra_labels: extra_labels }
if (klass = class_for_kind(definition["kind"]))
return klass.new(**opts)
end
Expand Down Expand Up @@ -115,14 +115,14 @@ def pretty_timeout_type
"timeout: #{timeout}s"
end

def initialize(namespace:, context:, definition:, logger:, statsd_tags: [])
def initialize(namespace:, context:, definition:, logger:, statsd_tags: [], extra_labels: {})
# subclasses must also set these if they define their own initializer
@name = (definition.dig("metadata", "name") || definition.dig("metadata", "generateName")).to_s
@optional_statsd_tags = statsd_tags
@namespace = namespace
@context = context
@logger = logger
@definition = definition
@definition = set_extra_labels(definition, extra_labels)
@statsd_report_done = false
@disappeared = false
@validation_errors = []
Expand Down Expand Up @@ -529,6 +529,14 @@ def krane_annotation_value(suffix)
@definition.dig("metadata", "annotations", Annotation.for(suffix))
end

def set_extra_labels(definition, extra_labels)
return definition if extra_labels.nil? || extra_labels.empty?
definition["metadata"] ||= {}
definition["metadata"]["labels"] ||= {}
definition["metadata"]["labels"].reverse_merge!(extra_labels)
definition
end

def validate_selector(selector)
if labels.nil?
@validation_errors << "selector #{selector} passed in, but no labels were defined"
Expand Down
4 changes: 2 additions & 2 deletions lib/krane/kubernetes_resource/custom_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ class CustomResource < KubernetesResource
(.metadata.generation != .status.observedGeneration).
MSG

def initialize(namespace:, context:, definition:, logger:, statsd_tags: [], crd:)
def initialize(namespace:, context:, definition:, logger:, statsd_tags: [], crd:, extra_labels: {})
super(namespace: namespace, context: context, definition: definition,
logger: logger, statsd_tags: statsd_tags)
logger: logger, statsd_tags: statsd_tags, extra_labels: extra_labels)
@crd = crd
end

Expand Down
4 changes: 2 additions & 2 deletions lib/krane/kubernetes_resource/pod.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class Pod < KubernetesResource
attr_reader :definition

def initialize(namespace:, context:, definition:, logger:,
statsd_tags: nil, parent: nil, deploy_started_at: nil, stream_logs: false)
statsd_tags: nil, parent: nil, deploy_started_at: nil, stream_logs: false, extra_labels: {})
@parent = parent
@deploy_started_at = deploy_started_at

Expand All @@ -25,7 +25,7 @@ def initialize(namespace:, context:, definition:, logger:,
@containers += definition["spec"].fetch("initContainers", []).map { |c| Container.new(c, init_container: true) }
@stream_logs = stream_logs
super(namespace: namespace, context: context, definition: definition,
logger: logger, statsd_tags: statsd_tags)
logger: logger, statsd_tags: statsd_tags, extra_labels: extra_labels)
end

def sync(_cache)
Expand Down
4 changes: 2 additions & 2 deletions lib/krane/kubernetes_resource/replica_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ class ReplicaSet < PodSetBase
attr_reader :pods

def initialize(namespace:, context:, definition:, logger:, statsd_tags: nil,
parent: nil, deploy_started_at: nil)
parent: nil, deploy_started_at: nil, extra_labels: {})
@parent = parent
@deploy_started_at = deploy_started_at
@pods = []
super(namespace: namespace, context: context, definition: definition,
logger: logger, statsd_tags: statsd_tags)
logger: logger, statsd_tags: statsd_tags, extra_labels: extra_labels)
end

def sync(cache)
Expand Down
2 changes: 1 addition & 1 deletion lib/krane/render_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class RenderTask
# @param current_sha [String] The SHA of the commit
# @param filenames [Array<String>] An array of filenames and/or directories containing templates (*required*)
# @param bindings [Hash] Bindings parsed by Krane::BindingsParser
def initialize(logger: nil, current_sha:, filenames: [], bindings:)
def initialize(logger: nil, current_sha:, filenames: [], bindings:, extra_labels: {})
@logger = logger || Krane::FormattedLogger.build
@filenames = filenames.map { |path| File.expand_path(path) }
@bindings = bindings
Expand Down
1 change: 1 addition & 0 deletions test/exe/deploy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ def default_options(new_args = {}, run_args = {})
selector: nil,
selector_as_filter: false,
protected_namespaces: ["default", "kube-system", "kube-public"],
extra_labels: nil,
}.merge(new_args),
run_args: {
verify_result: true,
Expand Down
1 change: 1 addition & 0 deletions test/exe/global_deploy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ def default_options(new_args = {}, run_args = {})
global_timeout: 300,
selector: 'name=web',
selector_as_filter: false,
extra_labels: nil,
}.merge(new_args),
run_args: {
verify_result: true,
Expand Down
32 changes: 32 additions & 0 deletions test/unit/krane/extra_labels_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true
require 'test_helper'
require 'krane/extra_labels'

class ExtraLabelsTest < ::Minitest::Test
def test_parse_extra_labels
expected = { "foo" => "42", "bar" => "17" }
assert_equal(expected, parse("foo=42,bar=17"))
end

def test_parse_extra_labels_with_equality_sign
expected = { "foo" => "1=2=3", "bar" => "3", "bla" => "4=7" }
assert_equal(expected, parse("foo=1=2=3,bar=3,bla=4=7"))
end

def test_parse_extra_labels_with_no_value
expected = { "bla" => nil, "foo" => "" }
assert_equal(expected, parse("bla,foo="))
end

def test_parse_extra_labels_with_no_key
assert_raises(ArgumentError, "key is blank") do
parse("=17,foo=42")
end
end

private

def parse(string)
Krane::ExtraLabels.parse(string).to_h
end
end
66 changes: 66 additions & 0 deletions test/unit/krane/kubernetes_resource/kubernetes_resource_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# frozen_string_literal: true
require 'test_helper'

class KubernetesResourceTest < Krane::TestCase
def test_extra_labels
[
{kind: "ConfigMap"},
{kind: "CronJob"},
{kind: "CustomResourceDefinition"},
{kind: "DaemonSet"},
{kind: "Deployment"},
{kind: "HorizontalPodAutoscaler"},
{kind: "Ingress"},
{kind: "Job"},
{kind: "NetworkPolicy"},
{kind: "PersistentVolumeClaim"},
{kind: "PodDisruptionBudget"},
{kind: "PodSetBase"},
{kind: "PodTemplate"},
{kind: "ReplicaSet"},
{kind: "ResourceQuota"},
{kind: "Role"},
{kind: "RoleBinding"},
{kind: "Secret"},
{kind: "Service"},
{kind: "ServiceAccount"},
{kind: "StatefulSet"},

{kind: "Pod", spec: {"containers" => [{"name" => "someContainer"}]}},
{kind: "SomeCustomResource", init_args: {crd: "SomeCRD"}},
{kind: "ResourceUnknownToKrane"},
].each do |resource|
args = {
namespace: 'test',
context: 'nope',
logger: @logger,
statsd_tags: [],
extra_labels: {
"extra" => "label",
"overwritten" => "yes"
},
definition: {
"kind" => resource.fetch(:kind),
"metadata" => {
"name" => "testsuite",
"labels" => {
"overwritten" => "no"
},
},
"spec" => resource.fetch(:spec, {})
}
}
args.merge!(resource.fetch(:init_args, {}))

resource = begin
Krane::KubernetesResource.build(**args)
rescue ArgumentError => e
flunk("failed to build #{resource.fetch(:kind)}: #{e.message}")
end

assert_equal(resource.send(:labels),
{"extra"=>"label", "overwritten"=>"no"},
"expected #{resource} to apply extra_labels")
end
end
end

0 comments on commit e848750

Please sign in to comment.