-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add support for namespace label selector to authn-k8s #2613
Changes from all commits
1faba26
9553766
070f233
31b1f6f
62ba804
dc0ad24
fb441de
1a7d45b
1083804
5d2bed7
80af822
35b9a18
fd41865
b6e4ff7
0012bb7
e7d1727
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,6 +104,15 @@ def pod_by_name(podname, namespace) | |
k8s_client_for_method("get_pod").get_pod(podname, namespace) | ||
end | ||
|
||
# Returns the labels hash for a Namespace with a given name. | ||
# | ||
# @return nil if no such Namespace exists. | ||
def namespace_labels_hash(namespace) | ||
namespace_object = k8s_client_for_method("get_namespace").get_namespace(namespace) | ||
|
||
return namespace_object.metadata.labels.to_h unless namespace_object.nil? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Authentication::AuthnK8s::K8sObjectLookup#namespace_labels_hash performs a nil-check |
||
end | ||
|
||
# Locates pods matching label selector in a namespace. | ||
# | ||
def pods_by_label(label_selector, namespace) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,44 @@ def valid_resource?(type:, name:) | |
@logger.debug(LogMessages::Authentication::AuthnK8s::ValidatedK8sResource.new(type, name)) | ||
end | ||
|
||
# Validates label selector and creates a hash | ||
# In the spirit of https://github.com/kubernetes/apimachinery/blob/master/pkg/labels/selector.go | ||
def valid_namespace?(label_selector:) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Method There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Complex method Authentication::AuthnK8s::K8sResourceValidator#valid_namespace? (33.5) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Complex method Authentication::AuthnK8s::K8sResourceValidator#valid_namespace? (40.4) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Authentication::AuthnK8s::K8sResourceValidator#valid_namespace? has approx 14 statements |
||
@logger.debug(LogMessages::Authentication::AuthnK8s::ValidatingK8sResourceLabel.new('namespace', namespace, label_selector)) | ||
|
||
if label_selector.length == 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
raise Errors::Authentication::AuthnK8s::InvalidLabelSelector.new(label_selector) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Authentication::AuthnK8s::K8sResourceValidator#valid_namespace? calls 'raise Errors::Authentication::AuthnK8s::InvalidLabelSelector.new(label_selector)' 2 times There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Provide an exception class and message as arguments to |
||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add empty line after guard clause. |
||
label_selector_hash = label_selector | ||
.split(",") | ||
.map{ |kv_pair| | ||
kv_pair = kv_pair.split(/={1,2}/, 2) | ||
|
||
invalid ||= kv_pair.length != 2 | ||
invalid ||= kv_pair[0].include?("!") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Authentication::AuthnK8s::K8sResourceValidator#valid_namespace? calls 'kv_pair[0]' 2 times |
||
|
||
if (invalid) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't use parentheses around a variable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't use parentheses around the condition of an |
||
raise Errors::Authentication::AuthnK8s::InvalidLabelSelector.new(label_selector) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Provide an exception class and message as arguments to |
||
end | ||
|
||
kv_pair[0] = kv_pair[0].to_sym | ||
kv_pair | ||
} | ||
.to_h | ||
|
||
# Fetch namespace labels | ||
# TODO: refactor this to have a generic label fetching method in @k8s_object_lookup | ||
labels_hash = @k8s_object_lookup.namespace_labels_hash(namespace) | ||
|
||
# Validates label selector hash against labels hash | ||
unless label_selector_hash.all? { |k, v| labels_hash[k] == v } | ||
raise Errors::Authentication::AuthnK8s::LabelSelectorMismatch.new('namespace', namespace, label_selector) | ||
end | ||
|
||
@logger.debug(LogMessages::Authentication::AuthnK8s::ValidatedK8sResourceLabel.new('namespace', namespace, label_selector)) | ||
return true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redundant |
||
end | ||
|
||
private | ||
|
||
def retrieve_k8s_resource(type, name) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
module Authentication | ||
module Constraints | ||
|
||
# This constraint is initialized with an array of strings. | ||
# They represent resource restrictions where exactly one is required. | ||
class RequiredExclusiveConstraint | ||
|
||
def initialize(required_exclusive:) | ||
@required_exclusive = required_exclusive | ||
end | ||
|
||
def validate(resource_restrictions:) | ||
restrictions_found = resource_restrictions & @required_exclusive | ||
raise Errors::Authentication::Constraints::IllegalRequiredExclusiveCombination.new(@required_exclusive, restrictions_found) unless restrictions_found.length == 1 | ||
end | ||
|
||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
#!/usr/bin/env bash | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to merge this script? Maybe this could be codified as a way of building appliance images for a target working/POC branches, but it seems like once this PR merges, we won't need it in its current form. |
||
set -euo pipefail | ||
|
||
|
||
IMAGE="registry.tld/conjur-appliance:eval-authn-k8s-label-selector" | ||
|
||
echo "Building on top of stable appliance image and pushing to ${IMAGE}" | ||
|
||
echo " | ||
|
||
# --- | ||
FROM registry.tld/conjur-appliance:5.0-stable | ||
|
||
# Copy new source files | ||
$( | ||
echo " | ||
app/domain/authentication/authn_k8s/authentication_request.rb | ||
app/domain/authentication/authn_k8s/consts.rb | ||
app/domain/authentication/authn_k8s/k8s_object_lookup.rb | ||
app/domain/authentication/authn_k8s/k8s_resource_validator.rb | ||
app/domain/authentication/constraints/required_exclusive_constraint.rb | ||
app/domain/errors.rb | ||
" | docker run --rm -i --entrypoint="" ruby:2-alpine ruby -e ' | ||
files = STDIN.read.split("\n").reject(&:empty?) | ||
puts files.map {|file| "COPY #{file} /opt/conjur/possum/#{file}"}.join("\n") | ||
' | ||
) | ||
|
||
RUN chown -R conjur:conjur /opt/conjur/possum/app | ||
|
||
# --- | ||
|
||
" | \ | ||
tee /dev/stderr | \ | ||
docker build -f - -t "${IMAGE}" . | ||
|
||
docker push "${IMAGE}" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
# frozen_string_literal: true | ||
|
||
require 'spec_helper' | ||
|
||
RSpec.describe(Authentication::AuthnK8s::K8sResourceValidator) do | ||
let(:log_output) { StringIO.new } | ||
let(:logger) { | ||
Logger.new( | ||
log_output, | ||
formatter: proc do | severity, time, progname, msg | | ||
"#{severity},#{msg}\n" | ||
end) | ||
} | ||
|
||
subject { | ||
described_class.new(k8s_object_lookup: k8s_object_lookup, pod: pod, logger: logger) | ||
} | ||
|
||
let(:k8s_object_lookup) { | ||
double("k8s_object_lookup").tap { |d| | ||
allow(d).to receive(:namespace_labels_hash) | ||
.with("namespace_name") | ||
.and_return({ :key1 => "value1", :key2 => "value2" }) | ||
} | ||
} | ||
|
||
let(:pod) { | ||
double("pod").tap { |d| | ||
d.stub_chain("metadata.namespace").and_return("namespace_name") | ||
} | ||
} | ||
|
||
context "#valid_namespace?" do | ||
it 'raises error on empty label selector' do | ||
expect { subject.valid_namespace?(label_selector: "") }.to( | ||
raise_error( | ||
::Errors::Authentication::AuthnK8s::InvalidLabelSelector | ||
) | ||
) | ||
end | ||
|
||
it 'raises error on invalid label selector' do | ||
# No key-value pair | ||
expect { subject.valid_namespace?(label_selector: "key,") }.to( | ||
raise_error( | ||
::Errors::Authentication::AuthnK8s::InvalidLabelSelector | ||
) | ||
) | ||
|
||
# Unsupported operator | ||
expect { subject.valid_namespace?(label_selector: "key!=value") }.to( | ||
raise_error( | ||
::Errors::Authentication::AuthnK8s::InvalidLabelSelector | ||
) | ||
) | ||
end | ||
|
||
it 'returns true for labels matching label-selector' do | ||
# Single key, single equals format | ||
expect( | ||
subject.valid_namespace?(label_selector: "key1=value1") | ||
).to be true | ||
# Single key, double equals format | ||
expect( | ||
subject.valid_namespace?(label_selector: "key2==value2") | ||
).to be true | ||
# Multiple keys | ||
expect( | ||
subject.valid_namespace?(label_selector: "key1=value1,key2=value2") | ||
).to be true | ||
end | ||
|
||
it 'throws an error for labels not matching label-selector' do | ||
# Value mismatch | ||
expect { subject.valid_namespace?(label_selector: "key1=notvalue") }.to( | ||
raise_error( | ||
::Errors::Authentication::AuthnK8s::LabelSelectorMismatch | ||
) | ||
) | ||
# Key not found | ||
expect { subject.valid_namespace?(label_selector: "notfoundkey=value") }.to( | ||
raise_error( | ||
::Errors::Authentication::AuthnK8s::LabelSelectorMismatch | ||
) | ||
) | ||
# One of multiple keys does not match | ||
expect { subject.valid_namespace?(label_selector: "key1=value1,notfoundkey=value") }.to( | ||
raise_error( | ||
::Errors::Authentication::AuthnK8s::LabelSelectorMismatch | ||
) | ||
) | ||
end | ||
|
||
it 'logs before label-selector validation begins, and after success' do | ||
subject.valid_namespace?(label_selector: "key1=value1") | ||
|
||
expect(log_output.string.split("\n")).to include( | ||
"DEBUG,CONJ00145D Validating K8s resource using label selector. Type:'namespace', Name:'namespace_name', Label:'key1=value1'", | ||
"DEBUG,CONJ00146D Validated K8s resource using label selector. Type:'namespace', Name:'namespace_name', Label:'key1=value1'" | ||
) | ||
end | ||
|
||
it 'logs before label-selector validation begins, but not after failure' do | ||
expect { subject.valid_namespace?(label_selector: "key1=notvalue") }.to raise_error | ||
|
||
expect(log_output.string.split("\n")).to include( | ||
"DEBUG,CONJ00145D Validating K8s resource using label selector. Type:'namespace', Name:'namespace_name', Label:'key1=notvalue'", | ||
) | ||
expect(log_output.string.split("\n")).not_to include( | ||
"DEBUG,CONJ00146D Validated K8s resource using label selector. Type:'namespace', Name:'namespace_name', Label:'key1=notvalue'" | ||
) | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Authentication::AuthnK8s::K8sObjectLookup#namespace_labels_hash refers to 'namespace_object' more than self (maybe move it to another class?)