Skip to content
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

Create conjur v5 k8s authenticator #619

Merged
merged 113 commits into from Jul 11, 2018

Conversation

jtuttle
Copy link
Member

@jtuttle jtuttle commented Jul 10, 2018

Closes CONJ-5128

What does this pull request do?

Ports the v4 Kubernetes authenticator to Conjur v5.

Where should the reviewer start?

The new inject_client_cert route in the authenticate_controller or perhaps the new kubernetes cuke tests.

How should this be manually tested?

Manually testing this will be quite a chore, but you can mimic what the test scripts are doing in ci/authn-k8s. A better way would be to update http://github.com/cyberark/kubernetes-conjur-deploy to work w/ Conjur v5.

Link to build in Jenkins (if appropriate)

https://jenkins.conjur.net/job/cyberark--conjur/job/CONJ-5128--create-conjur-v5-k8s-authenticator/

Questions:

Does this have automated Cucumber tests?

Yes

Can we make a blog post, video, or animated GIF out of this?

If you like watching text fly by in terminals, sure.

Is this explained in documentation?

Yes

This is to accomodate a lot of code imported wholesale
@ghost ghost assigned dividedmind Jul 11, 2018
John Tuttle added 2 commits July 11, 2018 10:09
@timeout = timeout
end

def exec command, body: "", stdin: false
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method exec has 76 lines of code (exceeds 25 allowed). Consider refactoring.

end
end

class StatefulSet < Base
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

class ReplicaSet < Base
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

Then(/^I( can)? authenticate pod matching "([^"]*)" with authn-k8s as "([^"]*)"( without cert and key)?$/) do |success, objectid, hostid, nocertkey|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@request_ip = find_matching_pod(objectid)
end

Then(/^I( can)? authenticate with authn-k8s as "([^"]*)"( without cert and key)?$/) do |success, objectid, nocertkey|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end

# Tests whether the Pod's ReplicaSet belongs to the Deployment.
class Deployment < Base
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

class DeploymentConfig < Base
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.


class StatefulSet < Base
def validate_pod
stateful_set_ref = verify "Pod #{pod.metadata.name.inspect} does not belong to a StatefulSet" do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Authentication::AuthnK8s::K8sResolver::StatefulSet#validate_pod calls 'pod.metadata.name' 2 times

@@ -0,0 +1,427 @@
module Authentication
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File authenticator.rb has 302 lines of code (exceeds 250 allowed). Consider refactoring.

end
end

def pod_name_login
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method pod_name_login has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.

end

# get pod cert
def pod_certificate
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method pod_certificate has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.

end

# Find a valid request IP for the given object.
def detect_request_ip objectid
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method detect_request_ip has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.


# In the case that this node is a follower, it's necessary to wait for the
# variables to be replicated from the master.
def wait_for_variable var, value
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method wait_for_variable has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.

@pod_name
end

def pod_name_authenticate
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method pod_name_authenticate has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.


# ssl stuff

def csr_spiffe_id(csr)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method csr_spiffe_id has a Cognitive Complexity of 17 (exceeds 5 allowed). Consider refactoring.

# authn-k8s AuthenticateController helpers
#----------------------------------------

def pod_certificate
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method pod_certificate has a Cognitive Complexity of 14 (exceeds 5 allowed). Consider refactoring.

host.annotations.find { |a| a.values[:name] == 'kubernetes/authentication-container-name' }[:value] || 'authenticator'
end

def find_pod
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method find_pod has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

["pod", "service_account", "deployment", "stateful_set", "deployment_config"].include? k8s_controller_name
end

def find_pod_under_controller
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method find_pod_under_controller has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.

class ClientCertExpiredError < RuntimeError; end
class NotFoundError < RuntimeError; end

class Authenticator
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Class Authenticator has 39 methods (exceeds 20 allowed). Consider refactoring.


# ssl stuff

def csr_spiffe_id(csr)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method csr_spiffe_id has 29 lines of code (exceeds 25 allowed). Consider refactoring.

@timeout = timeout
end

def exec command, body: "", stdin: false
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method exec has a Cognitive Complexity of 13 (exceeds 5 allowed). Consider refactoring.

resp
end

Then(/^I( can)? login to pod matching "([^"]*)" to authn-k8s as "([^"]*)"$/) do |success, objectid, host_id|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

Then(/^I( can)? login to authn-k8s as "([^"]*)"$/) do |success, objectid|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

def pod_name_login
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@pod_name
end

def pod_name_authenticate
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

resp
end

Then(/^I( can)? login to pod matching "([^"]*)" to authn-k8s as "([^"]*)"$/) do |success, objectid, host_id|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

Then(/^I( can)? login to authn-k8s as "([^"]*)"$/) do |success, objectid|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

def pod_name_login
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@pod_name
end

def pod_name_authenticate
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.


def login username, request_ip, authn_k8s_host, pkey
csr = gen_csr(username, pkey, [
"URI:spiffe://cluster.local/namespace/#{@pod.metadata.namespace}/pod/#{@pod.metadata.name}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

login calls '@pod.metadata' 2 times

resp
end

Then(/^I( can)? login to pod matching "([^"]*)" to authn-k8s as "([^"]*)"$/) do |success, objectid, host_id|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

Then(/^I( can)? login to authn-k8s as "([^"]*)"$/) do |success, objectid|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

def pod_name_login
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@pod_name
end

def pod_name_authenticate
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.


def login username, request_ip, authn_k8s_host, pkey
csr = gen_csr(username, pkey, [
"URI:spiffe://cluster.local/namespace/#{@pod.metadata.namespace}/pod/#{@pod.metadata.name}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

login calls '@pod.metadata' 2 times

@jvanderhoof jvanderhoof self-requested a review July 11, 2018 16:17
resp
end

Then(/^I( can)? login to pod matching "([^"]*)" to authn-k8s as "([^"]*)"$/) do |success, objectid, host_id|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

Then(/^I( can)? login to authn-k8s as "([^"]*)"$/) do |success, objectid|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

def pod_name_login
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@pod_name
end

def pod_name_authenticate
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.


def login username, request_ip, authn_k8s_host, pkey
csr = gen_csr(username, pkey, [
"URI:spiffe://cluster.local/namespace/#{@pod.metadata.namespace}/pod/#{@pod.metadata.name}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

login calls '@pod.metadata' 2 times

@ghost ghost assigned jvanderhoof Jul 11, 2018
resp
end

Then(/^I( can)? login to pod matching "([^"]*)" to authn-k8s as "([^"]*)"$/) do |success, objectid, host_id|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

Then(/^I( can)? login to authn-k8s as "([^"]*)"$/) do |success, objectid|
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

def pod_name_login
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@pod_name
end

def pod_name_authenticate
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.


def login username, request_ip, authn_k8s_host, pkey
csr = gen_csr(username, pkey, [
"URI:spiffe://cluster.local/namespace/#{@pod.metadata.namespace}/pod/#{@pod.metadata.name}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

login calls '@pod.metadata' 2 times

Copy link
Contributor

@jvanderhoof jvanderhoof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving this PR for now. Additional work will be completed to improve the quality.

@jvanderhoof jvanderhoof merged commit 1a3aa89 into master Jul 11, 2018
@ghost ghost removed the in progress label Jul 11, 2018
@jvanderhoof jvanderhoof deleted the CONJ-5128--create-conjur-v5-k8s-authenticator branch July 11, 2018 17:39
@jtuttle jtuttle changed the title WIP create conjur v5 k8s authenticator Create conjur v5 k8s authenticator Jul 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants