From 164f64a3706fc8754c58b9b6a2bb0eaf063e8847 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Thu, 17 Sep 2020 11:18:29 -0500 Subject: [PATCH 01/11] Add IdentityProvider field to TokenCredentialRequestSpec. Signed-off-by: Matt Moyer --- apis/login/types_token.go.tmpl | 8 +++++++- apis/login/v1alpha1/types_token.go.tmpl | 8 +++++++- generated/1.17/README.adoc | 1 + generated/1.17/apis/go.mod | 5 ++++- generated/1.17/apis/go.sum | 2 ++ generated/1.17/apis/login/types_token.go | 8 +++++++- generated/1.17/apis/login/v1alpha1/types_token.go | 8 +++++++- .../1.17/apis/login/v1alpha1/zz_generated.conversion.go | 2 ++ .../1.17/apis/login/v1alpha1/zz_generated.deepcopy.go | 3 ++- generated/1.17/apis/login/zz_generated.deepcopy.go | 3 ++- generated/1.17/client/openapi/zz_generated.openapi.go | 9 +++++++++ generated/1.18/README.adoc | 1 + generated/1.18/apis/go.mod | 5 ++++- generated/1.18/apis/go.sum | 2 ++ generated/1.18/apis/login/types_token.go | 8 +++++++- generated/1.18/apis/login/v1alpha1/types_token.go | 8 +++++++- .../1.18/apis/login/v1alpha1/zz_generated.conversion.go | 2 ++ .../1.18/apis/login/v1alpha1/zz_generated.deepcopy.go | 3 ++- generated/1.18/apis/login/zz_generated.deepcopy.go | 3 ++- generated/1.18/client/openapi/zz_generated.openapi.go | 9 +++++++++ generated/1.19/README.adoc | 1 + generated/1.19/apis/go.mod | 5 ++++- generated/1.19/apis/go.sum | 2 ++ generated/1.19/apis/login/types_token.go | 8 +++++++- generated/1.19/apis/login/v1alpha1/types_token.go | 8 +++++++- .../1.19/apis/login/v1alpha1/zz_generated.conversion.go | 2 ++ .../1.19/apis/login/v1alpha1/zz_generated.deepcopy.go | 3 ++- generated/1.19/apis/login/zz_generated.deepcopy.go | 3 ++- generated/1.19/client/openapi/zz_generated.openapi.go | 9 +++++++++ hack/lib/update-codegen.sh | 7 ++++--- 30 files changed, 126 insertions(+), 20 deletions(-) diff --git a/apis/login/types_token.go.tmpl b/apis/login/types_token.go.tmpl index 55b9fc99..91d36cfb 100644 --- a/apis/login/types_token.go.tmpl +++ b/apis/login/types_token.go.tmpl @@ -3,11 +3,17 @@ package login -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) type TokenCredentialRequestSpec struct { // Bearer token supplied with the credential request. Token string + + // Reference to an identity provider which can fulfill this credential request. + IdentityProvider corev1.TypedLocalObjectReference } type TokenCredentialRequestStatus struct { diff --git a/apis/login/v1alpha1/types_token.go.tmpl b/apis/login/v1alpha1/types_token.go.tmpl index 7580874f..9fba3369 100644 --- a/apis/login/v1alpha1/types_token.go.tmpl +++ b/apis/login/v1alpha1/types_token.go.tmpl @@ -3,12 +3,18 @@ package v1alpha1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // TokenCredentialRequestSpec is the specification of a TokenCredentialRequest, expected on requests to the Pinniped API. type TokenCredentialRequestSpec struct { // Bearer token supplied with the credential request. Token string `json:"token,omitempty"` + + // Reference to an identity provider which can fulfill this credential request. + IdentityProvider corev1.TypedLocalObjectReference `json:"identityProvider"` } // TokenCredentialRequestStatus is the status of a TokenCredentialRequest, returned on responses to the Pinniped API. diff --git a/generated/1.17/README.adoc b/generated/1.17/README.adoc index f9849700..0c2458df 100644 --- a/generated/1.17/README.adoc +++ b/generated/1.17/README.adoc @@ -263,6 +263,7 @@ TokenCredentialRequestSpec is the specification of a TokenCredentialRequest, exp |=== | Field | Description | *`token`* __string__ | Bearer token supplied with the credential request. +| *`identityProvider`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.17/#typedlocalobjectreference-v1-core[$$TypedLocalObjectReference$$]__ | Reference to an identity provider which can fulfill this credential request. |=== diff --git a/generated/1.17/apis/go.mod b/generated/1.17/apis/go.mod index dbe3440b..5104a8f1 100644 --- a/generated/1.17/apis/go.mod +++ b/generated/1.17/apis/go.mod @@ -3,4 +3,7 @@ module go.pinniped.dev/generated/1.17/apis go 1.13 -require k8s.io/apimachinery v0.17.11 +require ( + k8s.io/api v0.17.11 + k8s.io/apimachinery v0.17.11 +) diff --git a/generated/1.17/apis/go.sum b/generated/1.17/apis/go.sum index d361f58d..971929d4 100644 --- a/generated/1.17/apis/go.sum +++ b/generated/1.17/apis/go.sum @@ -94,6 +94,8 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +k8s.io/api v0.17.11 h1:FAO+RMv/JhjBawixa33qZNZ2Yb5lNjgGuK8IjN2Ac3s= +k8s.io/api v0.17.11/go.mod h1:WR3CbTwCAxtfMcEB6c92W3l5aZw09unPCyxmxjYV3xg= k8s.io/apimachinery v0.17.11 h1:hgMFLIR+ofBpaPb27lZkf44v3bLn3MLqcbnw32PgoGA= k8s.io/apimachinery v0.17.11/go.mod h1:q+iFxLyaMeWIBhSlQ4OMkvdwbwrb8Ux0ALl90XD9paU= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= diff --git a/generated/1.17/apis/login/types_token.go b/generated/1.17/apis/login/types_token.go index 55b9fc99..91d36cfb 100644 --- a/generated/1.17/apis/login/types_token.go +++ b/generated/1.17/apis/login/types_token.go @@ -3,11 +3,17 @@ package login -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) type TokenCredentialRequestSpec struct { // Bearer token supplied with the credential request. Token string + + // Reference to an identity provider which can fulfill this credential request. + IdentityProvider corev1.TypedLocalObjectReference } type TokenCredentialRequestStatus struct { diff --git a/generated/1.17/apis/login/v1alpha1/types_token.go b/generated/1.17/apis/login/v1alpha1/types_token.go index 7580874f..9fba3369 100644 --- a/generated/1.17/apis/login/v1alpha1/types_token.go +++ b/generated/1.17/apis/login/v1alpha1/types_token.go @@ -3,12 +3,18 @@ package v1alpha1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // TokenCredentialRequestSpec is the specification of a TokenCredentialRequest, expected on requests to the Pinniped API. type TokenCredentialRequestSpec struct { // Bearer token supplied with the credential request. Token string `json:"token,omitempty"` + + // Reference to an identity provider which can fulfill this credential request. + IdentityProvider corev1.TypedLocalObjectReference `json:"identityProvider"` } // TokenCredentialRequestStatus is the status of a TokenCredentialRequest, returned on responses to the Pinniped API. diff --git a/generated/1.17/apis/login/v1alpha1/zz_generated.conversion.go b/generated/1.17/apis/login/v1alpha1/zz_generated.conversion.go index f8e1b005..9e125488 100644 --- a/generated/1.17/apis/login/v1alpha1/zz_generated.conversion.go +++ b/generated/1.17/apis/login/v1alpha1/zz_generated.conversion.go @@ -157,6 +157,7 @@ func Convert_login_TokenCredentialRequestList_To_v1alpha1_TokenCredentialRequest func autoConvert_v1alpha1_TokenCredentialRequestSpec_To_login_TokenCredentialRequestSpec(in *TokenCredentialRequestSpec, out *login.TokenCredentialRequestSpec, s conversion.Scope) error { out.Token = in.Token + out.IdentityProvider = in.IdentityProvider return nil } @@ -167,6 +168,7 @@ func Convert_v1alpha1_TokenCredentialRequestSpec_To_login_TokenCredentialRequest func autoConvert_login_TokenCredentialRequestSpec_To_v1alpha1_TokenCredentialRequestSpec(in *login.TokenCredentialRequestSpec, out *TokenCredentialRequestSpec, s conversion.Scope) error { out.Token = in.Token + out.IdentityProvider = in.IdentityProvider return nil } diff --git a/generated/1.17/apis/login/v1alpha1/zz_generated.deepcopy.go b/generated/1.17/apis/login/v1alpha1/zz_generated.deepcopy.go index 439149d9..2001aa60 100644 --- a/generated/1.17/apis/login/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.17/apis/login/v1alpha1/zz_generated.deepcopy.go @@ -33,7 +33,7 @@ func (in *TokenCredentialRequest) DeepCopyInto(out *TokenCredentialRequest) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) return } @@ -92,6 +92,7 @@ func (in *TokenCredentialRequestList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TokenCredentialRequestSpec) DeepCopyInto(out *TokenCredentialRequestSpec) { *out = *in + in.IdentityProvider.DeepCopyInto(&out.IdentityProvider) return } diff --git a/generated/1.17/apis/login/zz_generated.deepcopy.go b/generated/1.17/apis/login/zz_generated.deepcopy.go index 176c0b05..d92ad253 100644 --- a/generated/1.17/apis/login/zz_generated.deepcopy.go +++ b/generated/1.17/apis/login/zz_generated.deepcopy.go @@ -33,7 +33,7 @@ func (in *TokenCredentialRequest) DeepCopyInto(out *TokenCredentialRequest) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) return } @@ -92,6 +92,7 @@ func (in *TokenCredentialRequestList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TokenCredentialRequestSpec) DeepCopyInto(out *TokenCredentialRequestSpec) { *out = *in + in.IdentityProvider.DeepCopyInto(&out.IdentityProvider) return } diff --git a/generated/1.17/client/openapi/zz_generated.openapi.go b/generated/1.17/client/openapi/zz_generated.openapi.go index 829e2448..83e7fb87 100644 --- a/generated/1.17/client/openapi/zz_generated.openapi.go +++ b/generated/1.17/client/openapi/zz_generated.openapi.go @@ -671,9 +671,18 @@ func schema_117_apis_login_v1alpha1_TokenCredentialRequestSpec(ref common.Refere Format: "", }, }, + "identityProvider": { + SchemaProps: spec.SchemaProps{ + Description: "Reference to an identity provider which can fulfill this credential request.", + Ref: ref("k8s.io/api/core/v1.TypedLocalObjectReference"), + }, + }, }, + Required: []string{"identityProvider"}, }, }, + Dependencies: []string{ + "k8s.io/api/core/v1.TypedLocalObjectReference"}, } } diff --git a/generated/1.18/README.adoc b/generated/1.18/README.adoc index 22cfe325..faf0ca70 100644 --- a/generated/1.18/README.adoc +++ b/generated/1.18/README.adoc @@ -263,6 +263,7 @@ TokenCredentialRequestSpec is the specification of a TokenCredentialRequest, exp |=== | Field | Description | *`token`* __string__ | Bearer token supplied with the credential request. +| *`identityProvider`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#typedlocalobjectreference-v1-core[$$TypedLocalObjectReference$$]__ | Reference to an identity provider which can fulfill this credential request. |=== diff --git a/generated/1.18/apis/go.mod b/generated/1.18/apis/go.mod index bceb5f18..6940a6e9 100644 --- a/generated/1.18/apis/go.mod +++ b/generated/1.18/apis/go.mod @@ -3,4 +3,7 @@ module go.pinniped.dev/generated/1.18/apis go 1.13 -require k8s.io/apimachinery v0.18.2 +require ( + k8s.io/api v0.18.2 + k8s.io/apimachinery v0.18.2 +) diff --git a/generated/1.18/apis/go.sum b/generated/1.18/apis/go.sum index 8fe7e2d5..4ee816ab 100644 --- a/generated/1.18/apis/go.sum +++ b/generated/1.18/apis/go.sum @@ -94,6 +94,8 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +k8s.io/api v0.18.2 h1:wG5g5ZmSVgm5B+eHMIbI9EGATS2L8Z72rda19RIEgY8= +k8s.io/api v0.18.2/go.mod h1:SJCWI7OLzhZSvbY7U8zwNl9UA4o1fizoug34OV/2r78= k8s.io/apimachinery v0.18.2 h1:44CmtbmkzVDAhCpRVSiP2R5PPrC2RtlIv/MoB8xpdRA= k8s.io/apimachinery v0.18.2/go.mod h1:9SnR/e11v5IbyPCGbvJViimtJ0SwHG4nfZFjU77ftcA= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= diff --git a/generated/1.18/apis/login/types_token.go b/generated/1.18/apis/login/types_token.go index 55b9fc99..91d36cfb 100644 --- a/generated/1.18/apis/login/types_token.go +++ b/generated/1.18/apis/login/types_token.go @@ -3,11 +3,17 @@ package login -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) type TokenCredentialRequestSpec struct { // Bearer token supplied with the credential request. Token string + + // Reference to an identity provider which can fulfill this credential request. + IdentityProvider corev1.TypedLocalObjectReference } type TokenCredentialRequestStatus struct { diff --git a/generated/1.18/apis/login/v1alpha1/types_token.go b/generated/1.18/apis/login/v1alpha1/types_token.go index 7580874f..9fba3369 100644 --- a/generated/1.18/apis/login/v1alpha1/types_token.go +++ b/generated/1.18/apis/login/v1alpha1/types_token.go @@ -3,12 +3,18 @@ package v1alpha1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // TokenCredentialRequestSpec is the specification of a TokenCredentialRequest, expected on requests to the Pinniped API. type TokenCredentialRequestSpec struct { // Bearer token supplied with the credential request. Token string `json:"token,omitempty"` + + // Reference to an identity provider which can fulfill this credential request. + IdentityProvider corev1.TypedLocalObjectReference `json:"identityProvider"` } // TokenCredentialRequestStatus is the status of a TokenCredentialRequest, returned on responses to the Pinniped API. diff --git a/generated/1.18/apis/login/v1alpha1/zz_generated.conversion.go b/generated/1.18/apis/login/v1alpha1/zz_generated.conversion.go index b3e37f56..fdf8c9f5 100644 --- a/generated/1.18/apis/login/v1alpha1/zz_generated.conversion.go +++ b/generated/1.18/apis/login/v1alpha1/zz_generated.conversion.go @@ -157,6 +157,7 @@ func Convert_login_TokenCredentialRequestList_To_v1alpha1_TokenCredentialRequest func autoConvert_v1alpha1_TokenCredentialRequestSpec_To_login_TokenCredentialRequestSpec(in *TokenCredentialRequestSpec, out *login.TokenCredentialRequestSpec, s conversion.Scope) error { out.Token = in.Token + out.IdentityProvider = in.IdentityProvider return nil } @@ -167,6 +168,7 @@ func Convert_v1alpha1_TokenCredentialRequestSpec_To_login_TokenCredentialRequest func autoConvert_login_TokenCredentialRequestSpec_To_v1alpha1_TokenCredentialRequestSpec(in *login.TokenCredentialRequestSpec, out *TokenCredentialRequestSpec, s conversion.Scope) error { out.Token = in.Token + out.IdentityProvider = in.IdentityProvider return nil } diff --git a/generated/1.18/apis/login/v1alpha1/zz_generated.deepcopy.go b/generated/1.18/apis/login/v1alpha1/zz_generated.deepcopy.go index 439149d9..2001aa60 100644 --- a/generated/1.18/apis/login/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.18/apis/login/v1alpha1/zz_generated.deepcopy.go @@ -33,7 +33,7 @@ func (in *TokenCredentialRequest) DeepCopyInto(out *TokenCredentialRequest) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) return } @@ -92,6 +92,7 @@ func (in *TokenCredentialRequestList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TokenCredentialRequestSpec) DeepCopyInto(out *TokenCredentialRequestSpec) { *out = *in + in.IdentityProvider.DeepCopyInto(&out.IdentityProvider) return } diff --git a/generated/1.18/apis/login/zz_generated.deepcopy.go b/generated/1.18/apis/login/zz_generated.deepcopy.go index 176c0b05..d92ad253 100644 --- a/generated/1.18/apis/login/zz_generated.deepcopy.go +++ b/generated/1.18/apis/login/zz_generated.deepcopy.go @@ -33,7 +33,7 @@ func (in *TokenCredentialRequest) DeepCopyInto(out *TokenCredentialRequest) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) return } @@ -92,6 +92,7 @@ func (in *TokenCredentialRequestList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TokenCredentialRequestSpec) DeepCopyInto(out *TokenCredentialRequestSpec) { *out = *in + in.IdentityProvider.DeepCopyInto(&out.IdentityProvider) return } diff --git a/generated/1.18/client/openapi/zz_generated.openapi.go b/generated/1.18/client/openapi/zz_generated.openapi.go index 1c15426f..ac54aea2 100644 --- a/generated/1.18/client/openapi/zz_generated.openapi.go +++ b/generated/1.18/client/openapi/zz_generated.openapi.go @@ -671,9 +671,18 @@ func schema_118_apis_login_v1alpha1_TokenCredentialRequestSpec(ref common.Refere Format: "", }, }, + "identityProvider": { + SchemaProps: spec.SchemaProps{ + Description: "Reference to an identity provider which can fulfill this credential request.", + Ref: ref("k8s.io/api/core/v1.TypedLocalObjectReference"), + }, + }, }, + Required: []string{"identityProvider"}, }, }, + Dependencies: []string{ + "k8s.io/api/core/v1.TypedLocalObjectReference"}, } } diff --git a/generated/1.19/README.adoc b/generated/1.19/README.adoc index cb788bdd..9e0a6579 100644 --- a/generated/1.19/README.adoc +++ b/generated/1.19/README.adoc @@ -263,6 +263,7 @@ TokenCredentialRequestSpec is the specification of a TokenCredentialRequest, exp |=== | Field | Description | *`token`* __string__ | Bearer token supplied with the credential request. +| *`identityProvider`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#typedlocalobjectreference-v1-core[$$TypedLocalObjectReference$$]__ | Reference to an identity provider which can fulfill this credential request. |=== diff --git a/generated/1.19/apis/go.mod b/generated/1.19/apis/go.mod index 5f704018..f515b351 100644 --- a/generated/1.19/apis/go.mod +++ b/generated/1.19/apis/go.mod @@ -3,4 +3,7 @@ module go.pinniped.dev/generated/1.19/apis go 1.13 -require k8s.io/apimachinery v0.19.0 +require ( + k8s.io/api v0.19.0 + k8s.io/apimachinery v0.19.0 +) diff --git a/generated/1.19/apis/go.sum b/generated/1.19/apis/go.sum index d442a122..61415065 100644 --- a/generated/1.19/apis/go.sum +++ b/generated/1.19/apis/go.sum @@ -157,6 +157,8 @@ gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= +k8s.io/api v0.19.0 h1:XyrFIJqTYZJ2DU7FBE/bSPz7b1HvbVBuBf07oeo6eTc= +k8s.io/api v0.19.0/go.mod h1:I1K45XlvTrDjmj5LoM5LuP/KYrhWbjUKT/SoPG0qTjw= k8s.io/apimachinery v0.19.0 h1:gjKnAda/HZp5k4xQYjL0K/Yb66IvNqjthCb03QlKpaQ= k8s.io/apimachinery v0.19.0/go.mod h1:DnPGDnARWFvYa3pMHgSxtbZb7gpzzAZ1pTfaUNDVlmA= k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= diff --git a/generated/1.19/apis/login/types_token.go b/generated/1.19/apis/login/types_token.go index 55b9fc99..91d36cfb 100644 --- a/generated/1.19/apis/login/types_token.go +++ b/generated/1.19/apis/login/types_token.go @@ -3,11 +3,17 @@ package login -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) type TokenCredentialRequestSpec struct { // Bearer token supplied with the credential request. Token string + + // Reference to an identity provider which can fulfill this credential request. + IdentityProvider corev1.TypedLocalObjectReference } type TokenCredentialRequestStatus struct { diff --git a/generated/1.19/apis/login/v1alpha1/types_token.go b/generated/1.19/apis/login/v1alpha1/types_token.go index 7580874f..9fba3369 100644 --- a/generated/1.19/apis/login/v1alpha1/types_token.go +++ b/generated/1.19/apis/login/v1alpha1/types_token.go @@ -3,12 +3,18 @@ package v1alpha1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // TokenCredentialRequestSpec is the specification of a TokenCredentialRequest, expected on requests to the Pinniped API. type TokenCredentialRequestSpec struct { // Bearer token supplied with the credential request. Token string `json:"token,omitempty"` + + // Reference to an identity provider which can fulfill this credential request. + IdentityProvider corev1.TypedLocalObjectReference `json:"identityProvider"` } // TokenCredentialRequestStatus is the status of a TokenCredentialRequest, returned on responses to the Pinniped API. diff --git a/generated/1.19/apis/login/v1alpha1/zz_generated.conversion.go b/generated/1.19/apis/login/v1alpha1/zz_generated.conversion.go index 2289296c..e2c6f280 100644 --- a/generated/1.19/apis/login/v1alpha1/zz_generated.conversion.go +++ b/generated/1.19/apis/login/v1alpha1/zz_generated.conversion.go @@ -157,6 +157,7 @@ func Convert_login_TokenCredentialRequestList_To_v1alpha1_TokenCredentialRequest func autoConvert_v1alpha1_TokenCredentialRequestSpec_To_login_TokenCredentialRequestSpec(in *TokenCredentialRequestSpec, out *login.TokenCredentialRequestSpec, s conversion.Scope) error { out.Token = in.Token + out.IdentityProvider = in.IdentityProvider return nil } @@ -167,6 +168,7 @@ func Convert_v1alpha1_TokenCredentialRequestSpec_To_login_TokenCredentialRequest func autoConvert_login_TokenCredentialRequestSpec_To_v1alpha1_TokenCredentialRequestSpec(in *login.TokenCredentialRequestSpec, out *TokenCredentialRequestSpec, s conversion.Scope) error { out.Token = in.Token + out.IdentityProvider = in.IdentityProvider return nil } diff --git a/generated/1.19/apis/login/v1alpha1/zz_generated.deepcopy.go b/generated/1.19/apis/login/v1alpha1/zz_generated.deepcopy.go index 439149d9..2001aa60 100644 --- a/generated/1.19/apis/login/v1alpha1/zz_generated.deepcopy.go +++ b/generated/1.19/apis/login/v1alpha1/zz_generated.deepcopy.go @@ -33,7 +33,7 @@ func (in *TokenCredentialRequest) DeepCopyInto(out *TokenCredentialRequest) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) return } @@ -92,6 +92,7 @@ func (in *TokenCredentialRequestList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TokenCredentialRequestSpec) DeepCopyInto(out *TokenCredentialRequestSpec) { *out = *in + in.IdentityProvider.DeepCopyInto(&out.IdentityProvider) return } diff --git a/generated/1.19/apis/login/zz_generated.deepcopy.go b/generated/1.19/apis/login/zz_generated.deepcopy.go index 176c0b05..d92ad253 100644 --- a/generated/1.19/apis/login/zz_generated.deepcopy.go +++ b/generated/1.19/apis/login/zz_generated.deepcopy.go @@ -33,7 +33,7 @@ func (in *TokenCredentialRequest) DeepCopyInto(out *TokenCredentialRequest) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) return } @@ -92,6 +92,7 @@ func (in *TokenCredentialRequestList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TokenCredentialRequestSpec) DeepCopyInto(out *TokenCredentialRequestSpec) { *out = *in + in.IdentityProvider.DeepCopyInto(&out.IdentityProvider) return } diff --git a/generated/1.19/client/openapi/zz_generated.openapi.go b/generated/1.19/client/openapi/zz_generated.openapi.go index 5a4cef84..c27ff8c0 100644 --- a/generated/1.19/client/openapi/zz_generated.openapi.go +++ b/generated/1.19/client/openapi/zz_generated.openapi.go @@ -672,9 +672,18 @@ func schema_119_apis_login_v1alpha1_TokenCredentialRequestSpec(ref common.Refere Format: "", }, }, + "identityProvider": { + SchemaProps: spec.SchemaProps{ + Description: "Reference to an identity provider which can fulfill this credential request.", + Ref: ref("k8s.io/api/core/v1.TypedLocalObjectReference"), + }, + }, }, + Required: []string{"identityProvider"}, }, }, + Dependencies: []string{ + "k8s.io/api/core/v1.TypedLocalObjectReference"}, } } diff --git a/hack/lib/update-codegen.sh b/hack/lib/update-codegen.sh index 57ef3d37..46ef6f09 100755 --- a/hack/lib/update-codegen.sh +++ b/hack/lib/update-codegen.sh @@ -79,6 +79,7 @@ go 1.13 require ( k8s.io/apimachinery ${KUBE_MODULE_VERSION} + k8s.io/api ${KUBE_MODULE_VERSION} ) EOF @@ -93,9 +94,9 @@ go 1.13 require ( github.com/go-openapi/spec v0.19.9 - k8s.io/api ${KUBE_MODULE_VERSION} - k8s.io/apimachinery ${KUBE_MODULE_VERSION} - k8s.io/client-go ${KUBE_MODULE_VERSION} + k8s.io/api ${KUBE_MODULE_VERSION} + k8s.io/apimachinery ${KUBE_MODULE_VERSION} + k8s.io/client-go ${KUBE_MODULE_VERSION} k8s.io/apimachinery ${KUBE_MODULE_VERSION} ) From fbe055142627462678dad30072df2e9d35a63d23 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Thu, 17 Sep 2020 17:11:47 -0500 Subject: [PATCH 02/11] Add IDP selector support in client code. Signed-off-by: Matt Moyer --- cmd/pinniped/cmd/exchange_credential.go | 38 +++++++++++++++++-- cmd/pinniped/cmd/exchange_credential_test.go | 31 ++++++++++++++-- internal/client/client.go | 10 +++-- internal/client/client_test.go | 39 +++++++++++++------- test/integration/client_test.go | 10 ++++- 5 files changed, 104 insertions(+), 24 deletions(-) diff --git a/cmd/pinniped/cmd/exchange_credential.go b/cmd/pinniped/cmd/exchange_credential.go index 33e2ddb1..1ad0470f 100644 --- a/cmd/pinniped/cmd/exchange_credential.go +++ b/cmd/pinniped/cmd/exchange_credential.go @@ -9,11 +9,14 @@ import ( "fmt" "io" "os" + "strings" "time" "github.com/spf13/cobra" + corev1 "k8s.io/api/core/v1" clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" + idpv1alpha1 "go.pinniped.dev/generated/1.19/apis/idp/v1alpha1" "go.pinniped.dev/internal/client" "go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/here" @@ -75,9 +78,19 @@ func newExchangeCredentialCmd(args []string, stdout, stderr io.Writer) *exchange } type envGetter func(string) (string, bool) -type tokenExchanger func(ctx context.Context, namespace, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) +type tokenExchanger func( + ctx context.Context, + namespace string, + idp corev1.TypedLocalObjectReference, + token string, + caBundle string, + apiEndpoint string, +) (*clientauthenticationv1beta1.ExecCredential, error) -const ErrMissingEnvVar = constable.Error("failed to get credential: environment variable not set") +const ( + ErrMissingEnvVar = constable.Error("failed to get credential: environment variable not set") + ErrInvalidIDPType = constable.Error("invalid IDP type") +) func runExchangeCredential(stdout, _ io.Writer) { err := exchangeCredential(os.LookupEnv, client.ExchangeToken, stdout, 30*time.Second) @@ -96,6 +109,16 @@ func exchangeCredential(envGetter envGetter, tokenExchanger tokenExchanger, outp return envVarNotSetError("PINNIPED_NAMESPACE") } + idpType, varExists := envGetter("PINNIPED_IDP_TYPE") + if !varExists { + return envVarNotSetError("PINNIPED_IDP_TYPE") + } + + idpName, varExists := envGetter("PINNIPED_IDP_NAME") + if !varExists { + return envVarNotSetError("PINNIPED_IDP_NAME") + } + token, varExists := envGetter("PINNIPED_TOKEN") if !varExists { return envVarNotSetError("PINNIPED_TOKEN") @@ -111,7 +134,16 @@ func exchangeCredential(envGetter envGetter, tokenExchanger tokenExchanger, outp return envVarNotSetError("PINNIPED_K8S_API_ENDPOINT") } - cred, err := tokenExchanger(ctx, namespace, token, caBundle, apiEndpoint) + idp := corev1.TypedLocalObjectReference{Name: idpName} + switch strings.ToLower(idpType) { + case "webhook": + idp.APIGroup = &idpv1alpha1.SchemeGroupVersion.Group + idp.Kind = "WebhookIdentityProvider" + default: + return fmt.Errorf(`%w: %q, supported values are "webhook"`, ErrInvalidIDPType, idpType) + } + + cred, err := tokenExchanger(ctx, namespace, idp, token, caBundle, apiEndpoint) if err != nil { return fmt.Errorf("failed to get credential: %w", err) } diff --git a/cmd/pinniped/cmd/exchange_credential_test.go b/cmd/pinniped/cmd/exchange_credential_test.go index eb7478f0..1d74f186 100644 --- a/cmd/pinniped/cmd/exchange_credential_test.go +++ b/cmd/pinniped/cmd/exchange_credential_test.go @@ -14,6 +14,7 @@ import ( "github.com/sclevine/spec" "github.com/sclevine/spec/report" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" @@ -136,6 +137,8 @@ func TestExchangeCredential(t *testing.T) { buffer = new(bytes.Buffer) fakeEnv = map[string]string{ "PINNIPED_NAMESPACE": "namespace from env", + "PINNIPED_IDP_TYPE": "Webhook", + "PINNIPED_IDP_NAME": "webhook name from env", "PINNIPED_TOKEN": "token from env", "PINNIPED_CA_BUNDLE": "ca bundle from env", "PINNIPED_K8S_API_ENDPOINT": "k8s api from env", @@ -149,6 +152,18 @@ func TestExchangeCredential(t *testing.T) { r.EqualError(err, "failed to get credential: environment variable not set: PINNIPED_NAMESPACE") }) + it("returns an error when PINNIPED_IDP_TYPE is missing", func() { + delete(fakeEnv, "PINNIPED_IDP_TYPE") + err := exchangeCredential(envGetter, tokenExchanger, buffer, 30*time.Second) + r.EqualError(err, "failed to get credential: environment variable not set: PINNIPED_IDP_TYPE") + }) + + it("returns an error when PINNIPED_IDP_NAME is missing", func() { + delete(fakeEnv, "PINNIPED_IDP_NAME") + err := exchangeCredential(envGetter, tokenExchanger, buffer, 30*time.Second) + r.EqualError(err, "failed to get credential: environment variable not set: PINNIPED_IDP_NAME") + }) + it("returns an error when PINNIPED_TOKEN is missing", func() { delete(fakeEnv, "PINNIPED_TOKEN") err := exchangeCredential(envGetter, tokenExchanger, buffer, 30*time.Second) @@ -168,9 +183,17 @@ func TestExchangeCredential(t *testing.T) { }) }) + when("env vars are invalid", func() { + it("returns an error when PINNIPED_IDP_TYPE is missing", func() { + fakeEnv["PINNIPED_IDP_TYPE"] = "invalid" + err := exchangeCredential(envGetter, tokenExchanger, buffer, 30*time.Second) + r.EqualError(err, `invalid IDP type: "invalid", supported values are "webhook"`) + }) + }) + when("the token exchange fails", func() { it.Before(func() { - tokenExchanger = func(ctx context.Context, namespace, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { + tokenExchanger = func(ctx context.Context, namespace string, idp corev1.TypedLocalObjectReference, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { return nil, fmt.Errorf("some error") } }) @@ -183,7 +206,7 @@ func TestExchangeCredential(t *testing.T) { when("the JSON encoder fails", func() { it.Before(func() { - tokenExchanger = func(ctx context.Context, namespace, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { + tokenExchanger = func(ctx context.Context, namespace string, idp corev1.TypedLocalObjectReference, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { return &clientauthenticationv1beta1.ExecCredential{ Status: &clientauthenticationv1beta1.ExecCredentialStatus{ Token: "some token", @@ -200,7 +223,7 @@ func TestExchangeCredential(t *testing.T) { when("the token exchange times out", func() { it.Before(func() { - tokenExchanger = func(ctx context.Context, namespace, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { + tokenExchanger = func(ctx context.Context, namespace string, idp corev1.TypedLocalObjectReference, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { select { case <-time.After(100 * time.Millisecond): return &clientauthenticationv1beta1.ExecCredential{ @@ -224,7 +247,7 @@ func TestExchangeCredential(t *testing.T) { var actualNamespace, actualToken, actualCaBundle, actualAPIEndpoint string it.Before(func() { - tokenExchanger = func(ctx context.Context, namespace, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { + tokenExchanger = func(ctx context.Context, namespace string, idp corev1.TypedLocalObjectReference, token, caBundle, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { actualNamespace, actualToken, actualCaBundle, actualAPIEndpoint = namespace, token, caBundle, apiEndpoint now := metav1.NewTime(time.Date(2020, 7, 29, 1, 2, 3, 0, time.UTC)) return &clientauthenticationv1beta1.ExecCredential{ diff --git a/internal/client/client.go b/internal/client/client.go index b88af506..06831cc8 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" "k8s.io/client-go/tools/clientcmd" @@ -21,15 +22,18 @@ import ( // ErrLoginFailed is returned by ExchangeToken when the server rejects the login request. var ErrLoginFailed = errors.New("login failed") -// ExchangeToken exchanges an opaque token using the Pinniped CredentialRequest API, returning a client-go ExecCredential valid on the target cluster. -func ExchangeToken(ctx context.Context, namespace string, token string, caBundle string, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { +// ExchangeToken exchanges an opaque token using the Pinniped TokenCredentialRequest API, returning a client-go ExecCredential valid on the target cluster. +func ExchangeToken(ctx context.Context, namespace string, idp corev1.TypedLocalObjectReference, token string, caBundle string, apiEndpoint string) (*clientauthenticationv1beta1.ExecCredential, error) { client, err := getClient(apiEndpoint, caBundle) if err != nil { return nil, fmt.Errorf("could not get API client: %w", err) } resp, err := client.LoginV1alpha1().TokenCredentialRequests(namespace).Create(ctx, &v1alpha1.TokenCredentialRequest{ - Spec: v1alpha1.TokenCredentialRequestSpec{Token: token}, + Spec: v1alpha1.TokenCredentialRequestSpec{ + Token: token, + IdentityProvider: idp, + }, }, metav1.CreateOptions{}) if err != nil { return nil, fmt.Errorf("could not login: %w", err) diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 8ba5ff66..482f588a 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -12,10 +12,12 @@ import ( "time" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientauthenticationv1beta1 "k8s.io/client-go/pkg/apis/clientauthentication/v1beta1" - "go.pinniped.dev/generated/1.19/apis/login/v1alpha1" + idpv1alpha1 "go.pinniped.dev/generated/1.19/apis/idp/v1alpha1" + loginv1alpha1 "go.pinniped.dev/generated/1.19/apis/login/v1alpha1" "go.pinniped.dev/internal/testutil" ) @@ -23,9 +25,15 @@ func TestExchangeToken(t *testing.T) { t.Parallel() ctx := context.Background() + testIDP := corev1.TypedLocalObjectReference{ + APIGroup: &idpv1alpha1.SchemeGroupVersion.Group, + Kind: "WebhookIdentityProvider", + Name: "test-webhook", + } + t.Run("invalid configuration", func(t *testing.T) { t.Parallel() - got, err := ExchangeToken(ctx, "test-namespace", "", "", "") + got, err := ExchangeToken(ctx, "test-namespace", testIDP, "", "", "") require.EqualError(t, err, "could not get API client: invalid configuration: no configuration has been provided, try setting KUBERNETES_MASTER environment variable") require.Nil(t, got) }) @@ -38,7 +46,7 @@ func TestExchangeToken(t *testing.T) { _, _ = w.Write([]byte("some server error")) }) - got, err := ExchangeToken(ctx, "test-namespace", "", caBundle, endpoint) + got, err := ExchangeToken(ctx, "test-namespace", testIDP, "", caBundle, endpoint) require.EqualError(t, err, `could not login: an error on the server ("some server error") has prevented the request from succeeding (post tokencredentialrequests.login.pinniped.dev)`) require.Nil(t, got) }) @@ -49,13 +57,13 @@ func TestExchangeToken(t *testing.T) { errorMessage := "some login failure" caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") - _ = json.NewEncoder(w).Encode(&v1alpha1.TokenCredentialRequest{ + _ = json.NewEncoder(w).Encode(&loginv1alpha1.TokenCredentialRequest{ TypeMeta: metav1.TypeMeta{APIVersion: "login.pinniped.dev/v1alpha1", Kind: "TokenCredentialRequest"}, - Status: v1alpha1.TokenCredentialRequestStatus{Message: &errorMessage}, + Status: loginv1alpha1.TokenCredentialRequestStatus{Message: &errorMessage}, }) }) - got, err := ExchangeToken(ctx, "test-namespace", "", caBundle, endpoint) + got, err := ExchangeToken(ctx, "test-namespace", testIDP, "", caBundle, endpoint) require.EqualError(t, err, `login failed: some login failure`) require.Nil(t, got) }) @@ -65,12 +73,12 @@ func TestExchangeToken(t *testing.T) { // Start a test server that returns without any error message but also without valid credentials caBundle, endpoint := testutil.TLSTestServer(t, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("content-type", "application/json") - _ = json.NewEncoder(w).Encode(&v1alpha1.TokenCredentialRequest{ + _ = json.NewEncoder(w).Encode(&loginv1alpha1.TokenCredentialRequest{ TypeMeta: metav1.TypeMeta{APIVersion: "login.pinniped.dev/v1alpha1", Kind: "TokenCredentialRequest"}, }) }) - got, err := ExchangeToken(ctx, "test-namespace", "", caBundle, endpoint) + got, err := ExchangeToken(ctx, "test-namespace", testIDP, "", caBundle, endpoint) require.EqualError(t, err, `login failed: unknown`) require.Nil(t, got) }) @@ -95,7 +103,12 @@ func TestExchangeToken(t *testing.T) { "creationTimestamp": null }, "spec": { - "token": "test-token" + "token": "test-token", + "identityProvider": { + "apiGroup": "idp.pinniped.dev", + "kind": "WebhookIdentityProvider", + "name": "test-webhook" + } }, "status": {} }`, @@ -103,10 +116,10 @@ func TestExchangeToken(t *testing.T) { ) w.Header().Set("content-type", "application/json") - _ = json.NewEncoder(w).Encode(&v1alpha1.TokenCredentialRequest{ + _ = json.NewEncoder(w).Encode(&loginv1alpha1.TokenCredentialRequest{ TypeMeta: metav1.TypeMeta{APIVersion: "login.pinniped.dev/v1alpha1", Kind: "TokenCredentialRequest"}, - Status: v1alpha1.TokenCredentialRequestStatus{ - Credential: &v1alpha1.ClusterCredential{ + Status: loginv1alpha1.TokenCredentialRequestStatus{ + Credential: &loginv1alpha1.ClusterCredential{ ExpirationTimestamp: expires, ClientCertificateData: "test-certificate", ClientKeyData: "test-key", @@ -115,7 +128,7 @@ func TestExchangeToken(t *testing.T) { }) }) - got, err := ExchangeToken(ctx, "test-namespace", "test-token", caBundle, endpoint) + got, err := ExchangeToken(ctx, "test-namespace", testIDP, "test-token", caBundle, endpoint) require.NoError(t, err) require.Equal(t, &clientauthenticationv1beta1.ExecCredential{ TypeMeta: metav1.TypeMeta{ diff --git a/test/integration/client_test.go b/test/integration/client_test.go index 9992126d..1c451e2d 100644 --- a/test/integration/client_test.go +++ b/test/integration/client_test.go @@ -10,7 +10,9 @@ import ( "time" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + idpv1alpha1 "go.pinniped.dev/generated/1.19/apis/idp/v1alpha1" "go.pinniped.dev/internal/client" "go.pinniped.dev/internal/here" "go.pinniped.dev/test/library" @@ -68,7 +70,13 @@ func TestClient(t *testing.T) { // Using the CA bundle and host from the current (admin) kubeconfig, do the token exchange. clientConfig := library.NewClientConfig(t) - resp, err := client.ExchangeToken(ctx, namespace, token, string(clientConfig.CAData), clientConfig.Host) + + idp := corev1.TypedLocalObjectReference{ + APIGroup: &idpv1alpha1.SchemeGroupVersion.Group, + Kind: "WebhookIdentityProvider", + Name: "pinniped-webhook", + } + resp, err := client.ExchangeToken(ctx, namespace, idp, token, string(clientConfig.CAData), clientConfig.Host) require.NoError(t, err) require.NotNil(t, resp.Status.ExpirationTimestamp) require.InDelta(t, time.Until(resp.Status.ExpirationTimestamp.Time), 1*time.Hour, float64(3*time.Minute)) From 6cdd4a950610fd2906ec053edbe2e8e1f18b49b0 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 21 Sep 2020 11:37:54 -0500 Subject: [PATCH 03/11] Add support for multiple IDPs selected using IdentityProvider field. This also has fallback compatibility support if no IDP is specified and there is exactly one IDP in the cache. Signed-off-by: Matt Moyer --- internal/apiserver/apiserver.go | 5 +- .../identityprovider/idpcache/cache.go | 103 +++-- .../identityprovider/idpcache/cache_test.go | 253 +++++++++---- .../webhookcachecleaner.go | 5 +- .../webhookcachecleaner_test.go | 54 ++- .../webhookcachefiller/webhookcachefiller.go | 7 +- .../credentialrequestmocks.go | 96 +++++ .../mocks/credentialrequestmocks/generate.go | 6 + internal/mocks/mockcertissuer/generate.go | 6 - .../mocks/mockcertissuer/mockcertissuer.go | 56 --- internal/registry/credentialrequest/rest.go | 56 ++- .../registry/credentialrequest/rest_test.go | 356 ++++-------------- internal/server/server.go | 9 +- 13 files changed, 519 insertions(+), 493 deletions(-) create mode 100644 internal/mocks/credentialrequestmocks/credentialrequestmocks.go create mode 100644 internal/mocks/credentialrequestmocks/generate.go delete mode 100644 internal/mocks/mockcertissuer/generate.go delete mode 100644 internal/mocks/mockcertissuer/mockcertissuer.go diff --git a/internal/apiserver/apiserver.go b/internal/apiserver/apiserver.go index 2cd3ce57..98d2c7bd 100644 --- a/internal/apiserver/apiserver.go +++ b/internal/apiserver/apiserver.go @@ -12,7 +12,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/registry/rest" genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/client-go/pkg/version" @@ -54,7 +53,7 @@ type Config struct { } type ExtraConfig struct { - TokenAuthenticator authenticator.Token + Authenticator credentialrequest.TokenCredentialRequestAuthenticator Issuer credentialrequest.CertIssuer StartControllersPostStartHook func(ctx context.Context) } @@ -98,7 +97,7 @@ func (c completedConfig) New() (*PinnipedServer, error) { } gvr := loginv1alpha1.SchemeGroupVersion.WithResource("tokencredentialrequests") - storage := credentialrequest.NewREST(c.ExtraConfig.TokenAuthenticator, c.ExtraConfig.Issuer) + storage := credentialrequest.NewREST(c.ExtraConfig.Authenticator, c.ExtraConfig.Issuer) if err := s.GenericAPIServer.InstallAPIGroup(&genericapiserver.APIGroupInfo{ PrioritizedVersions: []schema.GroupVersion{gvr.GroupVersion()}, VersionedResourcesStorageMap: map[string]map[string]rest.Storage{gvr.Version: {gvr.Resource: storage}}, diff --git a/internal/controller/identityprovider/idpcache/cache.go b/internal/controller/identityprovider/idpcache/cache.go index fa2480f0..ddbe63c7 100644 --- a/internal/controller/identityprovider/idpcache/cache.go +++ b/internal/controller/identityprovider/idpcache/cache.go @@ -10,15 +10,19 @@ import ( "sync" "k8s.io/apiserver/pkg/authentication/authenticator" + "k8s.io/apiserver/pkg/authentication/user" - "go.pinniped.dev/internal/controllerlib" + loginapi "go.pinniped.dev/generated/1.19/apis/login" ) var ( - // ErrNoIDPs is returned by Cache.AuthenticateToken() when there are no IDPs configured. + // ErrNoSuchIDP is returned by Cache.AuthenticateTokenCredentialRequest() when the requested IDP is not configured. + ErrNoSuchIDP = fmt.Errorf("no such identity provider") + + // ErrNoIDPs is returned by Cache.AuthenticateTokenCredentialRequest() when there are no IDPs configured. ErrNoIDPs = fmt.Errorf("no identity providers are loaded") - // ErrIndeterminateIDP is returned by Cache.AuthenticateToken() when the correct IDP cannot be determined. + // ErrIndeterminateIDP is returned by Cache.AuthenticateTokenCredentialRequest() when the correct IDP cannot be determined. ErrIndeterminateIDP = fmt.Errorf("could not uniquely match against an identity provider") ) @@ -28,48 +32,101 @@ type Cache struct { cache sync.Map } +type Key struct { + APIGroup string + Kind string + Namespace string + Name string +} + +type Value interface { + authenticator.Token +} + // New returns an empty cache. func New() *Cache { return &Cache{} } +// Get an identity provider by key. +func (c *Cache) Get(key Key) Value { + res, _ := c.cache.Load(key) + if res == nil { + return nil + } + return res.(Value) +} + // Store an identity provider into the cache. -func (c *Cache) Store(key controllerlib.Key, value authenticator.Token) { +func (c *Cache) Store(key Key, value Value) { c.cache.Store(key, value) } // Delete an identity provider from the cache. -func (c *Cache) Delete(key controllerlib.Key) { +func (c *Cache) Delete(key Key) { c.cache.Delete(key) } // Keys currently stored in the cache. -func (c *Cache) Keys() []controllerlib.Key { - var result []controllerlib.Key +func (c *Cache) Keys() []Key { + var result []Key c.cache.Range(func(key, _ interface{}) bool { - result = append(result, key.(controllerlib.Key)) + result = append(result, key.(Key)) return true }) return result } -// AuthenticateToken validates the provided token against the currently loaded identity providers. -func (c *Cache) AuthenticateToken(ctx context.Context, token string) (*authenticator.Response, bool, error) { - var matchingIDPs []authenticator.Token - c.cache.Range(func(key, value interface{}) bool { - matchingIDPs = append(matchingIDPs, value.(authenticator.Token)) - return true - }) - - // Return an error if there are no known IDPs. - if len(matchingIDPs) == 0 { - return nil, false, ErrNoIDPs +func (c *Cache) AuthenticateTokenCredentialRequest(ctx context.Context, req *loginapi.TokenCredentialRequest) (user.Info, error) { + // Map the incoming request to a cache key. + key := Key{ + Namespace: req.Namespace, + Name: req.Spec.IdentityProvider.Name, + Kind: req.Spec.IdentityProvider.Kind, + } + if req.Spec.IdentityProvider.APIGroup != nil { + key.APIGroup = *req.Spec.IdentityProvider.APIGroup } - // For now, allow there to be only exactly one IDP (until we specify a good mechanism for selecting one). - if len(matchingIDPs) != 1 { - return nil, false, ErrIndeterminateIDP + // If the IDP is unspecified (legacy requests), choose the single loaded IDP or fail if there is not exactly + // one IDP configured. + if key.Name == "" || key.Kind == "" || key.APIGroup == "" { + keys := c.Keys() + if len(keys) == 0 { + return nil, ErrNoIDPs + } + if len(keys) > 1 { + return nil, ErrIndeterminateIDP + } + key = keys[0] } - return matchingIDPs[0].AuthenticateToken(ctx, token) + val := c.Get(key) + if val == nil { + return nil, ErrNoSuchIDP + } + + // The incoming context could have an audience. Since we do not want to handle audiences right now, do not pass it + // through directly to the authentication webhook. + ctx = valuelessContext{ctx} + + // Call the selected IDP. + resp, authenticated, err := val.AuthenticateToken(ctx, req.Spec.Token) + if err != nil { + return nil, err + } + if !authenticated { + return nil, nil + } + + // Return the user.Info from the response (if it is non-nil). + var respUser user.Info + if resp != nil { + respUser = resp.User + } + return respUser, nil } + +type valuelessContext struct{ context.Context } + +func (valuelessContext) Value(interface{}) interface{} { return nil } diff --git a/internal/controller/identityprovider/idpcache/cache_test.go b/internal/controller/identityprovider/idpcache/cache_test.go index 65c1fd4e..33d156da 100644 --- a/internal/controller/identityprovider/idpcache/cache_test.go +++ b/internal/controller/identityprovider/idpcache/cache_test.go @@ -5,95 +5,212 @@ package idpcache import ( "context" + "fmt" "testing" + "time" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" - "go.pinniped.dev/internal/controllerlib" + idpv1alpha "go.pinniped.dev/generated/1.19/apis/idp/v1alpha1" + loginapi "go.pinniped.dev/generated/1.19/apis/login" "go.pinniped.dev/internal/mocks/mocktokenauthenticator" ) func TestCache(t *testing.T) { t.Parallel() - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + ctrl := gomock.NewController(t) + defer ctrl.Finish() - tests := []struct { - name string - mockAuthenticators map[controllerlib.Key]func(*mocktokenauthenticator.MockToken) - wantResponse *authenticator.Response - wantAuthenticated bool - wantErr string - }{ - { - name: "no IDPs", - wantErr: "no identity providers are loaded", - }, - { - name: "multiple IDPs", - mockAuthenticators: map[controllerlib.Key]func(mockToken *mocktokenauthenticator.MockToken){ - controllerlib.Key{Namespace: "foo", Name: "idp-one"}: nil, - controllerlib.Key{Namespace: "foo", Name: "idp-two"}: nil, - }, - wantErr: "could not uniquely match against an identity provider", - }, - { - name: "success", - mockAuthenticators: map[controllerlib.Key]func(mockToken *mocktokenauthenticator.MockToken){ - controllerlib.Key{ - Namespace: "foo", - Name: "idp-one", - }: func(mockToken *mocktokenauthenticator.MockToken) { - mockToken.EXPECT().AuthenticateToken(ctx, "test-token").Return( - &authenticator.Response{User: &user.DefaultInfo{Name: "test-user"}}, - true, - nil, - ) - }, - }, - wantResponse: &authenticator.Response{User: &user.DefaultInfo{Name: "test-user"}}, - wantAuthenticated: true, - }, + cache := New() + require.NotNil(t, cache) + + key1 := Key{Namespace: "foo", Name: "idp-one"} + mockToken1 := mocktokenauthenticator.NewMockToken(ctrl) + cache.Store(key1, mockToken1) + require.Equal(t, mockToken1, cache.Get(key1)) + require.Equal(t, 1, len(cache.Keys())) + + key2 := Key{Namespace: "foo", Name: "idp-two"} + mockToken2 := mocktokenauthenticator.NewMockToken(ctrl) + cache.Store(key2, mockToken2) + require.Equal(t, mockToken2, cache.Get(key2)) + require.Equal(t, 2, len(cache.Keys())) + + for _, key := range cache.Keys() { + cache.Delete(key) } - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() + require.Zero(t, len(cache.Keys())) +} +func TestAuthenticateTokenCredentialRequest(t *testing.T) { + t.Parallel() + + t.Run("missing IDP selector", func(t *testing.T) { + t.Run("no IDPs", func(t *testing.T) { + c := New() + res, err := c.AuthenticateTokenCredentialRequest(context.Background(), &loginapi.TokenCredentialRequest{}) + require.EqualError(t, err, "no identity providers are loaded") + require.Nil(t, res) + }) + + t.Run("multiple IDPs", func(t *testing.T) { + c := New() + c.Store(Key{Name: "idp-one"}, nil) + c.Store(Key{Name: "idp-two"}, nil) + res, err := c.AuthenticateTokenCredentialRequest(context.Background(), &loginapi.TokenCredentialRequest{}) + require.EqualError(t, err, "could not uniquely match against an identity provider") + require.Nil(t, res) + }) + + t.Run("single IDP", func(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - cache := New() - require.NotNil(t, cache) - require.Implements(t, (*authenticator.Token)(nil), cache) + c := New() + mockToken := mocktokenauthenticator.NewMockToken(ctrl) + mockToken.EXPECT().AuthenticateToken(gomock.Any(), "test-token"). + Return(&authenticator.Response{User: &user.DefaultInfo{Name: "test-user"}}, true, nil) + c.Store(Key{Name: "idp-one"}, mockToken) - for key, mockFunc := range tt.mockAuthenticators { - mockToken := mocktokenauthenticator.NewMockToken(ctrl) - if mockFunc != nil { - mockFunc(mockToken) - } - cache.Store(key, mockToken) - } - - require.Equal(t, len(tt.mockAuthenticators), len(cache.Keys())) - - resp, authenticated, err := cache.AuthenticateToken(ctx, "test-token") - require.Equal(t, tt.wantResponse, resp) - require.Equal(t, tt.wantAuthenticated, authenticated) - if tt.wantErr != "" { - require.EqualError(t, err, tt.wantErr) - return - } + res, err := c.AuthenticateTokenCredentialRequest(context.Background(), &loginapi.TokenCredentialRequest{ + Spec: loginapi.TokenCredentialRequestSpec{Token: "test-token"}, + }) require.NoError(t, err) - - for _, key := range cache.Keys() { - cache.Delete(key) - } - require.Zero(t, len(cache.Keys())) + require.Equal(t, "test-user", res.GetName()) }) + }) + + validRequest := loginapi.TokenCredentialRequest{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + }, + Spec: loginapi.TokenCredentialRequestSpec{ + IdentityProvider: corev1.TypedLocalObjectReference{ + APIGroup: &idpv1alpha.SchemeGroupVersion.Group, + Kind: "WebhookIdentityProvider", + Name: "test-name", + }, + Token: "test-token", + }, + Status: loginapi.TokenCredentialRequestStatus{}, } + validRequestKey := Key{ + APIGroup: *validRequest.Spec.IdentityProvider.APIGroup, + Kind: validRequest.Spec.IdentityProvider.Kind, + Namespace: validRequest.Namespace, + Name: validRequest.Spec.IdentityProvider.Name, + } + + mockCache := func(t *testing.T, res *authenticator.Response, authenticated bool, err error) *Cache { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + m := mocktokenauthenticator.NewMockToken(ctrl) + m.EXPECT().AuthenticateToken(audienceFreeContext{}, validRequest.Spec.Token).Return(res, authenticated, err) + c := New() + c.Store(validRequestKey, m) + return c + } + + t.Run("no such IDP", func(t *testing.T) { + c := New() + res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy()) + require.EqualError(t, err, "no such identity provider") + require.Nil(t, res) + }) + + t.Run("authenticator returns error", func(t *testing.T) { + c := mockCache(t, nil, false, fmt.Errorf("some authenticator error")) + res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy()) + require.EqualError(t, err, "some authenticator error") + require.Nil(t, res) + }) + + t.Run("authenticator returns unauthenticated without error", func(t *testing.T) { + c := mockCache(t, &authenticator.Response{}, false, nil) + res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy()) + require.NoError(t, err) + require.Nil(t, res) + }) + + t.Run("authenticator returns nil response without error", func(t *testing.T) { + c := mockCache(t, nil, true, nil) + res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy()) + require.NoError(t, err) + require.Nil(t, res) + }) + + t.Run("authenticator returns response with nil user", func(t *testing.T) { + c := mockCache(t, &authenticator.Response{}, true, nil) + res, err := c.AuthenticateTokenCredentialRequest(context.Background(), validRequest.DeepCopy()) + require.NoError(t, err) + require.Nil(t, res) + }) + + t.Run("context is cancelled", func(t *testing.T) { + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + m := mocktokenauthenticator.NewMockToken(ctrl) + m.EXPECT().AuthenticateToken(gomock.Any(), validRequest.Spec.Token).DoAndReturn( + func(ctx context.Context, token string) (*authenticator.Response, bool, error) { + select { + case <-time.After(2 * time.Second): + require.Fail(t, "expected to be cancelled") + return nil, true, nil + case <-ctx.Done(): + return nil, false, ctx.Err() + } + }, + ) + c := New() + c.Store(validRequestKey, m) + + ctx, cancel := context.WithCancel(context.Background()) + errchan := make(chan error) + go func() { + _, err := c.AuthenticateTokenCredentialRequest(ctx, validRequest.DeepCopy()) + errchan <- err + }() + cancel() + require.EqualError(t, <-errchan, "context canceled") + }) + + t.Run("authenticator returns success", func(t *testing.T) { + userInfo := user.DefaultInfo{ + Name: "test-user", + UID: "test-uid", + Groups: []string{"test-group-1", "test-group-2"}, + Extra: map[string][]string{"extra-key-1": {"extra-value-1", "extra-value-2"}}, + } + c := mockCache(t, &authenticator.Response{User: &userInfo}, true, nil) + + audienceCtx := authenticator.WithAudiences(context.Background(), authenticator.Audiences{"test-audience-1"}) + res, err := c.AuthenticateTokenCredentialRequest(audienceCtx, validRequest.DeepCopy()) + require.NoError(t, err) + require.NotNil(t, res) + require.Equal(t, "test-user", res.GetName()) + require.Equal(t, "test-uid", res.GetUID()) + require.Equal(t, []string{"test-group-1", "test-group-2"}, res.GetGroups()) + require.Equal(t, map[string][]string{"extra-key-1": {"extra-value-1", "extra-value-2"}}, res.GetExtra()) + }) +} + +type audienceFreeContext struct{} + +func (audienceFreeContext) Matches(in interface{}) bool { + ctx, isCtx := in.(context.Context) + if !isCtx { + return false + } + _, hasAudiences := authenticator.AudiencesFrom(ctx) + return !hasAudiences +} + +func (audienceFreeContext) String() string { + return "is a context without authenticator audiences" } diff --git a/internal/controller/identityprovider/webhookcachecleaner/webhookcachecleaner.go b/internal/controller/identityprovider/webhookcachecleaner/webhookcachecleaner.go index a4d71da6..c383a18a 100644 --- a/internal/controller/identityprovider/webhookcachecleaner/webhookcachecleaner.go +++ b/internal/controller/identityprovider/webhookcachecleaner/webhookcachecleaner.go @@ -59,7 +59,10 @@ func (c *controller) Sync(ctx controllerlib.Context) error { // Delete any entries from the cache which are no longer in the cluster. for _, key := range c.cache.Keys() { - if _, exists := webhooksByKey[key]; !exists { + if key.APIGroup != idpv1alpha1.SchemeGroupVersion.Group || key.Kind != "WebhookIdentityProvider" { + continue + } + if _, exists := webhooksByKey[controllerlib.Key{Namespace: key.Namespace, Name: key.Name}]; !exists { c.log.WithValues("idp", klog.KRef(key.Namespace, key.Name)).Info("deleting webhook IDP from cache") c.cache.Delete(key) } diff --git a/internal/controller/identityprovider/webhookcachecleaner/webhookcachecleaner_test.go b/internal/controller/identityprovider/webhookcachecleaner/webhookcachecleaner_test.go index 94bc15fb..70ea0197 100644 --- a/internal/controller/identityprovider/webhookcachecleaner/webhookcachecleaner_test.go +++ b/internal/controller/identityprovider/webhookcachecleaner/webhookcachecleaner_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apiserver/pkg/authentication/authenticator" idpv1alpha "go.pinniped.dev/generated/1.19/apis/idp/v1alpha1" pinnipedfake "go.pinniped.dev/generated/1.19/client/clientset/versioned/fake" @@ -24,22 +23,36 @@ import ( func TestController(t *testing.T) { t.Parallel() - testKey1 := controllerlib.Key{Namespace: "test-namespace", Name: "test-name-one"} - testKey2 := controllerlib.Key{Namespace: "test-namespace", Name: "test-name-two"} + testKey1 := idpcache.Key{ + APIGroup: "idp.pinniped.dev", + Kind: "WebhookIdentityProvider", + Namespace: "test-namespace", + Name: "test-name-one", + } + testKey2 := idpcache.Key{ + APIGroup: "idp.pinniped.dev", + Kind: "WebhookIdentityProvider", + Namespace: "test-namespace", + Name: "test-name-two", + } + testKeyNonwebhook := idpcache.Key{ + APIGroup: "idp.pinniped.dev", + Kind: "SomeOtherIdentityProvider", + Namespace: "test-namespace", + Name: "test-name-one", + } tests := []struct { name string - syncKey controllerlib.Key webhookIDPs []runtime.Object - initialCache map[controllerlib.Key]authenticator.Token + initialCache map[idpcache.Key]idpcache.Value wantErr string wantLogs []string - wantCacheKeys []controllerlib.Key + wantCacheKeys []idpcache.Key }{ { name: "no change", - syncKey: testKey1, - initialCache: map[controllerlib.Key]authenticator.Token{testKey1: nil}, + initialCache: map[idpcache.Key]idpcache.Value{testKey1: nil}, webhookIDPs: []runtime.Object{ &idpv1alpha.WebhookIdentityProvider{ ObjectMeta: metav1.ObjectMeta{ @@ -48,11 +61,10 @@ func TestController(t *testing.T) { }, }, }, - wantCacheKeys: []controllerlib.Key{testKey1}, + wantCacheKeys: []idpcache.Key{testKey1}, }, { name: "IDPs not yet added", - syncKey: testKey1, initialCache: nil, webhookIDPs: []runtime.Object{ &idpv1alpha.WebhookIdentityProvider{ @@ -68,14 +80,14 @@ func TestController(t *testing.T) { }, }, }, - wantCacheKeys: []controllerlib.Key{}, + wantCacheKeys: []idpcache.Key{}, }, { - name: "successful cleanup", - syncKey: testKey1, - initialCache: map[controllerlib.Key]authenticator.Token{ - testKey1: nil, - testKey2: nil, + name: "successful cleanup", + initialCache: map[idpcache.Key]idpcache.Value{ + testKey1: nil, + testKey2: nil, + testKeyNonwebhook: nil, }, webhookIDPs: []runtime.Object{ &idpv1alpha.WebhookIdentityProvider{ @@ -88,7 +100,7 @@ func TestController(t *testing.T) { wantLogs: []string{ `webhookcachecleaner-controller "level"=0 "msg"="deleting webhook IDP from cache" "idp"={"name":"test-name-two","namespace":"test-namespace"}`, }, - wantCacheKeys: []controllerlib.Key{testKey1}, + wantCacheKeys: []idpcache.Key{testKey1, testKeyNonwebhook}, }, } for _, tt := range tests { @@ -112,7 +124,13 @@ func TestController(t *testing.T) { informers.Start(ctx.Done()) controllerlib.TestRunSynchronously(t, controller) - syncCtx := controllerlib.Context{Context: ctx, Key: tt.syncKey} + syncCtx := controllerlib.Context{ + Context: ctx, + Key: controllerlib.Key{ + Namespace: "test-namespace", + Name: "test-name-one", + }, + } if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantErr != "" { require.EqualError(t, err, tt.wantErr) diff --git a/internal/controller/identityprovider/webhookcachefiller/webhookcachefiller.go b/internal/controller/identityprovider/webhookcachefiller/webhookcachefiller.go index b0343c58..e741a8b3 100644 --- a/internal/controller/identityprovider/webhookcachefiller/webhookcachefiller.go +++ b/internal/controller/identityprovider/webhookcachefiller/webhookcachefiller.go @@ -68,7 +68,12 @@ func (c *controller) Sync(ctx controllerlib.Context) error { return fmt.Errorf("failed to build webhook config: %w", err) } - c.cache.Store(ctx.Key, webhookAuthenticator) + c.cache.Store(idpcache.Key{ + APIGroup: idpv1alpha1.GroupName, + Kind: "WebhookIdentityProvider", + Namespace: ctx.Key.Namespace, + Name: ctx.Key.Name, + }, webhookAuthenticator) c.log.WithValues("idp", klog.KObj(obj), "endpoint", obj.Spec.Endpoint).Info("added new webhook IDP") return nil } diff --git a/internal/mocks/credentialrequestmocks/credentialrequestmocks.go b/internal/mocks/credentialrequestmocks/credentialrequestmocks.go new file mode 100644 index 00000000..c21e78d7 --- /dev/null +++ b/internal/mocks/credentialrequestmocks/credentialrequestmocks.go @@ -0,0 +1,96 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +// + +// Code generated by MockGen. DO NOT EDIT. +// Source: go.pinniped.dev/internal/registry/credentialrequest (interfaces: CertIssuer,TokenCredentialRequestAuthenticator) + +// Package credentialrequestmocks is a generated GoMock package. +package credentialrequestmocks + +import ( + context "context" + pkix "crypto/x509/pkix" + gomock "github.com/golang/mock/gomock" + login "go.pinniped.dev/generated/1.19/apis/login" + user "k8s.io/apiserver/pkg/authentication/user" + reflect "reflect" + time "time" +) + +// MockCertIssuer is a mock of CertIssuer interface +type MockCertIssuer struct { + ctrl *gomock.Controller + recorder *MockCertIssuerMockRecorder +} + +// MockCertIssuerMockRecorder is the mock recorder for MockCertIssuer +type MockCertIssuerMockRecorder struct { + mock *MockCertIssuer +} + +// NewMockCertIssuer creates a new mock instance +func NewMockCertIssuer(ctrl *gomock.Controller) *MockCertIssuer { + mock := &MockCertIssuer{ctrl: ctrl} + mock.recorder = &MockCertIssuerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockCertIssuer) EXPECT() *MockCertIssuerMockRecorder { + return m.recorder +} + +// IssuePEM mocks base method +func (m *MockCertIssuer) IssuePEM(arg0 pkix.Name, arg1 []string, arg2 time.Duration) ([]byte, []byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IssuePEM", arg0, arg1, arg2) + ret0, _ := ret[0].([]byte) + ret1, _ := ret[1].([]byte) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// IssuePEM indicates an expected call of IssuePEM +func (mr *MockCertIssuerMockRecorder) IssuePEM(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IssuePEM", reflect.TypeOf((*MockCertIssuer)(nil).IssuePEM), arg0, arg1, arg2) +} + +// MockTokenCredentialRequestAuthenticator is a mock of TokenCredentialRequestAuthenticator interface +type MockTokenCredentialRequestAuthenticator struct { + ctrl *gomock.Controller + recorder *MockTokenCredentialRequestAuthenticatorMockRecorder +} + +// MockTokenCredentialRequestAuthenticatorMockRecorder is the mock recorder for MockTokenCredentialRequestAuthenticator +type MockTokenCredentialRequestAuthenticatorMockRecorder struct { + mock *MockTokenCredentialRequestAuthenticator +} + +// NewMockTokenCredentialRequestAuthenticator creates a new mock instance +func NewMockTokenCredentialRequestAuthenticator(ctrl *gomock.Controller) *MockTokenCredentialRequestAuthenticator { + mock := &MockTokenCredentialRequestAuthenticator{ctrl: ctrl} + mock.recorder = &MockTokenCredentialRequestAuthenticatorMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockTokenCredentialRequestAuthenticator) EXPECT() *MockTokenCredentialRequestAuthenticatorMockRecorder { + return m.recorder +} + +// AuthenticateTokenCredentialRequest mocks base method +func (m *MockTokenCredentialRequestAuthenticator) AuthenticateTokenCredentialRequest(arg0 context.Context, arg1 *login.TokenCredentialRequest) (user.Info, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AuthenticateTokenCredentialRequest", arg0, arg1) + ret0, _ := ret[0].(user.Info) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AuthenticateTokenCredentialRequest indicates an expected call of AuthenticateTokenCredentialRequest +func (mr *MockTokenCredentialRequestAuthenticatorMockRecorder) AuthenticateTokenCredentialRequest(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthenticateTokenCredentialRequest", reflect.TypeOf((*MockTokenCredentialRequestAuthenticator)(nil).AuthenticateTokenCredentialRequest), arg0, arg1) +} diff --git a/internal/mocks/credentialrequestmocks/generate.go b/internal/mocks/credentialrequestmocks/generate.go new file mode 100644 index 00000000..c22ec978 --- /dev/null +++ b/internal/mocks/credentialrequestmocks/generate.go @@ -0,0 +1,6 @@ +// Copyright 2020 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package credentialrequestmocks + +//go:generate go run -v github.com/golang/mock/mockgen -destination=credentialrequestmocks.go -package=credentialrequestmocks -copyright_file=../../../hack/header.txt go.pinniped.dev/internal/registry/credentialrequest CertIssuer,TokenCredentialRequestAuthenticator diff --git a/internal/mocks/mockcertissuer/generate.go b/internal/mocks/mockcertissuer/generate.go deleted file mode 100644 index 82e74840..00000000 --- a/internal/mocks/mockcertissuer/generate.go +++ /dev/null @@ -1,6 +0,0 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package mockcertissuer - -//go:generate go run -v github.com/golang/mock/mockgen -destination=mockcertissuer.go -package=mockcertissuer -copyright_file=../../../hack/header.txt go.pinniped.dev/internal/registry/credentialrequest CertIssuer diff --git a/internal/mocks/mockcertissuer/mockcertissuer.go b/internal/mocks/mockcertissuer/mockcertissuer.go deleted file mode 100644 index e68383c1..00000000 --- a/internal/mocks/mockcertissuer/mockcertissuer.go +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2020 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 -// - -// Code generated by MockGen. DO NOT EDIT. -// Source: go.pinniped.dev/internal/registry/credentialrequest (interfaces: CertIssuer) - -// Package mockcertissuer is a generated GoMock package. -package mockcertissuer - -import ( - pkix "crypto/x509/pkix" - reflect "reflect" - time "time" - - gomock "github.com/golang/mock/gomock" -) - -// MockCertIssuer is a mock of CertIssuer interface -type MockCertIssuer struct { - ctrl *gomock.Controller - recorder *MockCertIssuerMockRecorder -} - -// MockCertIssuerMockRecorder is the mock recorder for MockCertIssuer -type MockCertIssuerMockRecorder struct { - mock *MockCertIssuer -} - -// NewMockCertIssuer creates a new mock instance -func NewMockCertIssuer(ctrl *gomock.Controller) *MockCertIssuer { - mock := &MockCertIssuer{ctrl: ctrl} - mock.recorder = &MockCertIssuerMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use -func (m *MockCertIssuer) EXPECT() *MockCertIssuerMockRecorder { - return m.recorder -} - -// IssuePEM mocks base method -func (m *MockCertIssuer) IssuePEM(arg0 pkix.Name, arg1 []string, arg2 time.Duration) ([]byte, []byte, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IssuePEM", arg0, arg1, arg2) - ret0, _ := ret[0].([]byte) - ret1, _ := ret[1].([]byte) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 -} - -// IssuePEM indicates an expected call of IssuePEM -func (mr *MockCertIssuerMockRecorder) IssuePEM(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IssuePEM", reflect.TypeOf((*MockCertIssuer)(nil).IssuePEM), arg0, arg1, arg2) -} diff --git a/internal/registry/credentialrequest/rest.go b/internal/registry/credentialrequest/rest.go index f8ccafb0..b3e31eab 100644 --- a/internal/registry/credentialrequest/rest.go +++ b/internal/registry/credentialrequest/rest.go @@ -14,7 +14,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" - "k8s.io/apiserver/pkg/authentication/authenticator" + "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/utils/trace" @@ -35,16 +35,20 @@ type CertIssuer interface { IssuePEM(subject pkix.Name, dnsNames []string, ttl time.Duration) ([]byte, []byte, error) } -func NewREST(tokenAuthenticator authenticator.Token, issuer CertIssuer) *REST { +type TokenCredentialRequestAuthenticator interface { + AuthenticateTokenCredentialRequest(ctx context.Context, req *loginapi.TokenCredentialRequest) (user.Info, error) +} + +func NewREST(authenticator TokenCredentialRequestAuthenticator, issuer CertIssuer) *REST { return &REST{ - tokenAuthenticator: tokenAuthenticator, - issuer: issuer, + authenticator: authenticator, + issuer: issuer, } } type REST struct { - tokenAuthenticator authenticator.Token - issuer CertIssuer + authenticator TokenCredentialRequestAuthenticator + issuer CertIssuer } func (*REST) New() runtime.Object { @@ -67,35 +71,20 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation return nil, err } - // The incoming context could have an audience. Since we do not want to handle audiences right now, do not pass it - // through directly to the authentication webhook. Instead only propagate cancellation of the parent context. - cancelCtx, cancel := context.WithCancel(context.Background()) - defer cancel() - go func() { - select { - case <-ctx.Done(): - cancel() - case <-cancelCtx.Done(): - } - }() - - authResponse, authenticated, err := r.tokenAuthenticator.AuthenticateToken(cancelCtx, credentialRequest.Spec.Token) + user, err := r.authenticator.AuthenticateTokenCredentialRequest(ctx, credentialRequest) if err != nil { traceFailureWithError(t, "webhook authentication", err) return failureResponse(), nil } - if !authenticated || authResponse == nil || authResponse.User == nil || authResponse.User.GetName() == "" { - traceSuccess(t, authResponse, authenticated, false) + if user == nil || user.GetName() == "" { + traceSuccess(t, user, false) return failureResponse(), nil } - username := authResponse.User.GetName() - groups := authResponse.User.GetGroups() - certPEM, keyPEM, err := r.issuer.IssuePEM( pkix.Name{ - CommonName: username, - Organization: groups, + CommonName: user.GetName(), + Organization: user.GetGroups(), }, []string{}, clientCertificateTTL, @@ -105,7 +94,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation return failureResponse(), nil } - traceSuccess(t, authResponse, authenticated, true) + traceSuccess(t, user, true) return &loginapi.TokenCredentialRequest{ Status: loginapi.TokenCredentialRequestStatus{ @@ -121,8 +110,8 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation func validateRequest(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions, t *trace.Trace) (*loginapi.TokenCredentialRequest, error) { credentialRequest, ok := obj.(*loginapi.TokenCredentialRequest) if !ok { - traceValidationFailure(t, "not a CredentialRequest") - return nil, apierrors.NewBadRequest(fmt.Sprintf("not a CredentialRequest: %#v", obj)) + traceValidationFailure(t, "not a TokenCredentialRequest") + return nil, apierrors.NewBadRequest(fmt.Sprintf("not a TokenCredentialRequest: %#v", obj)) } if len(credentialRequest.Spec.Token) == 0 { @@ -157,15 +146,14 @@ func validateRequest(ctx context.Context, obj runtime.Object, createValidation r return credentialRequest, nil } -func traceSuccess(t *trace.Trace, response *authenticator.Response, webhookAuthenticated bool, pinnipedAuthenticated bool) { +func traceSuccess(t *trace.Trace, userInfo user.Info, authenticated bool) { userID := "" - if response != nil && response.User != nil { - userID = response.User.GetUID() + if userInfo != nil { + userID = userInfo.GetUID() } t.Step("success", trace.Field{Key: "userID", Value: userID}, - trace.Field{Key: "idpAuthenticated", Value: webhookAuthenticated}, - trace.Field{Key: "pinnipedAuthenticated", Value: pinnipedAuthenticated}, + trace.Field{Key: "authenticated", Value: authenticated}, ) } diff --git a/internal/registry/credentialrequest/rest_test.go b/internal/registry/credentialrequest/rest_test.go index efa3071a..e44eae1b 100644 --- a/internal/registry/credentialrequest/rest_test.go +++ b/internal/registry/credentialrequest/rest_test.go @@ -17,45 +17,21 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/klog/v2" loginapi "go.pinniped.dev/generated/1.19/apis/login" - "go.pinniped.dev/internal/mocks/mockcertissuer" + "go.pinniped.dev/internal/mocks/credentialrequestmocks" "go.pinniped.dev/internal/testutil" ) -type contextKey struct{} - -type FakeToken struct { - calledWithToken string - calledWithContext context.Context - timeout time.Duration - reachedTimeout bool - cancelled bool - webhookStartedRunningNotificationChan chan bool - returnResponse *authenticator.Response - returnUnauthenticated bool - returnErr error -} - -func (f *FakeToken) AuthenticateToken(ctx context.Context, token string) (*authenticator.Response, bool, error) { - f.calledWithToken = token - f.calledWithContext = ctx - if f.webhookStartedRunningNotificationChan != nil { - f.webhookStartedRunningNotificationChan <- true - } - afterCh := time.After(f.timeout) - select { - case <-afterCh: - f.reachedTimeout = true - case <-ctx.Done(): - f.cancelled = true - } - return f.returnResponse, !f.returnUnauthenticated, f.returnErr +func TestNew(t *testing.T) { + r := NewREST(nil, nil) + require.NotNil(t, r) + require.True(t, r.NamespaceScoped()) + require.IsType(t, &loginapi.TokenCredentialRequest{}, r.New()) } func TestCreate(t *testing.T) { @@ -77,18 +53,17 @@ func TestCreate(t *testing.T) { }) it("CreateSucceedsWhenGivenATokenAndTheWebhookAuthenticatesTheToken", func() { - webhook := FakeToken{ - returnResponse: &authenticator.Response{ - User: &user.DefaultInfo{ - Name: "test-user", - UID: "test-user-uid", - Groups: []string{"test-group-1", "test-group-2"}, - }, - }, - returnUnauthenticated: false, - } + req := validCredentialRequest() - issuer := mockcertissuer.NewMockCertIssuer(ctrl) + requestAuthenticator := credentialrequestmocks.NewMockTokenCredentialRequestAuthenticator(ctrl) + requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req). + Return(&user.DefaultInfo{ + Name: "test-user", + UID: "test-user-uid", + Groups: []string{"test-group-1", "test-group-2"}, + }, nil) + + issuer := credentialrequestmocks.NewMockCertIssuer(ctrl) issuer.EXPECT().IssuePEM( pkix.Name{ CommonName: "test-user", @@ -97,10 +72,9 @@ func TestCreate(t *testing.T) { 1*time.Hour, ).Return([]byte("test-cert"), []byte("test-key"), nil) - storage := NewREST(&webhook, issuer) - requestToken := "a token" + storage := NewREST(requestAuthenticator, issuer) - response, err := callCreate(context.Background(), storage, validCredentialRequestWithToken(requestToken)) + response, err := callCreate(context.Background(), storage, req) r.NoError(err) r.IsType(&loginapi.TokenCredentialRequest{}, response) @@ -119,203 +93,89 @@ func TestCreate(t *testing.T) { }, }, }) - r.Equal(requestToken, webhook.calledWithToken) - requireOneLogStatement(r, logger, `"success" userID:test-user-uid,idpAuthenticated:true`) - }) - - it("CreateSucceedsWhenGivenANewLoginAPITokenAndTheWebhookAuthenticatesTheToken", func() { - webhook := FakeToken{ - returnResponse: &authenticator.Response{ - User: &user.DefaultInfo{ - Name: "test-user", - UID: "test-user-uid", - Groups: []string{"test-group-1", "test-group-2"}, - }, - }, - returnUnauthenticated: false, - } - - issuer := mockcertissuer.NewMockCertIssuer(ctrl) - issuer.EXPECT().IssuePEM( - pkix.Name{ - CommonName: "test-user", - Organization: []string{"test-group-1", "test-group-2"}}, - []string{}, - 1*time.Hour, - ).Return([]byte("test-cert"), []byte("test-key"), nil) - - storage := NewREST(&webhook, issuer) - requestToken := "a token" - - response, err := callCreate(context.Background(), storage, &loginapi.TokenCredentialRequest{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: "request name", - }, - Spec: loginapi.TokenCredentialRequestSpec{ - Token: requestToken, - }, - }) - - r.NoError(err) - r.IsType(&loginapi.TokenCredentialRequest{}, response) - - expires := response.(*loginapi.TokenCredentialRequest).Status.Credential.ExpirationTimestamp - r.NotNil(expires) - r.InDelta(time.Now().Add(1*time.Hour).Unix(), expires.Unix(), 5) - response.(*loginapi.TokenCredentialRequest).Status.Credential.ExpirationTimestamp = metav1.Time{} - - r.Equal(response, &loginapi.TokenCredentialRequest{ - Status: loginapi.TokenCredentialRequestStatus{ - Credential: &loginapi.ClusterCredential{ - ExpirationTimestamp: metav1.Time{}, - ClientCertificateData: "test-cert", - ClientKeyData: "test-key", - }, - }, - }) - r.Equal(requestToken, webhook.calledWithToken) - requireOneLogStatement(r, logger, `"success" userID:test-user-uid,idpAuthenticated:true`) + requireOneLogStatement(r, logger, `"success" userID:test-user-uid,authenticated:true`) }) it("CreateFailsWithValidTokenWhenCertIssuerFails", func() { - webhook := FakeToken{ - returnResponse: &authenticator.Response{ - User: &user.DefaultInfo{ - Name: "test-user", - Groups: []string{"test-group-1", "test-group-2"}, - }, - }, - returnUnauthenticated: false, - } + req := validCredentialRequest() - issuer := mockcertissuer.NewMockCertIssuer(ctrl) + requestAuthenticator := credentialrequestmocks.NewMockTokenCredentialRequestAuthenticator(ctrl) + requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req). + Return(&user.DefaultInfo{ + Name: "test-user", + Groups: []string{"test-group-1", "test-group-2"}, + }, nil) + + issuer := credentialrequestmocks.NewMockCertIssuer(ctrl) issuer.EXPECT(). IssuePEM(gomock.Any(), gomock.Any(), gomock.Any()). Return(nil, nil, fmt.Errorf("some certificate authority error")) - storage := NewREST(&webhook, issuer) - requestToken := "a token" + storage := NewREST(requestAuthenticator, issuer) - response, err := callCreate(context.Background(), storage, validCredentialRequestWithToken(requestToken)) + response, err := callCreate(context.Background(), storage, req) requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response) - r.Equal(requestToken, webhook.calledWithToken) requireOneLogStatement(r, logger, `"failure" failureType:cert issuer,msg:some certificate authority error`) }) - it("CreateSucceedsWithAnUnauthenticatedStatusWhenGivenATokenAndTheWebhookReturnsUnauthenticatedWithUserId", func() { - webhook := FakeToken{ - returnResponse: &authenticator.Response{ - User: &user.DefaultInfo{UID: "test-user-uid"}, - }, - returnUnauthenticated: true, - } - storage := NewREST(&webhook, nil) - requestToken := "a token" + it("CreateSucceedsWithAnUnauthenticatedStatusWhenGivenATokenAndTheWebhookReturnsNilUser", func() { + req := validCredentialRequest() - response, err := callCreate(context.Background(), storage, validCredentialRequestWithToken(requestToken)) + requestAuthenticator := credentialrequestmocks.NewMockTokenCredentialRequestAuthenticator(ctrl) + requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req).Return(nil, nil) + + storage := NewREST(requestAuthenticator, nil) + + response, err := callCreate(context.Background(), storage, req) requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response) - r.Equal(requestToken, webhook.calledWithToken) - requireOneLogStatement(r, logger, `"success" userID:test-user-uid,idpAuthenticated:false,pinnipedAuthenticated:false`) - }) - - it("CreateSucceedsWithAnUnauthenticatedStatusWhenGivenATokenAndTheWebhookReturnsUnauthenticatedWithNilUser", func() { - webhook := FakeToken{ - returnResponse: &authenticator.Response{User: nil}, - returnUnauthenticated: true, - } - storage := NewREST(&webhook, nil) - requestToken := "a token" - - response, err := callCreate(context.Background(), storage, validCredentialRequestWithToken(requestToken)) - - requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response) - r.Equal(requestToken, webhook.calledWithToken) - requireOneLogStatement(r, logger, `"success" userID:,idpAuthenticated:false,pinnipedAuthenticated:false`) + requireOneLogStatement(r, logger, `"success" userID:,authenticated:false`) }) it("CreateSucceedsWithAnUnauthenticatedStatusWhenWebhookFails", func() { - webhook := FakeToken{ - returnErr: errors.New("some webhook error"), - } - storage := NewREST(&webhook, nil) + req := validCredentialRequest() - response, err := callCreate(context.Background(), storage, validCredentialRequest()) + requestAuthenticator := credentialrequestmocks.NewMockTokenCredentialRequestAuthenticator(ctrl) + requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req). + Return(nil, errors.New("some webhook error")) + + storage := NewREST(requestAuthenticator, nil) + + response, err := callCreate(context.Background(), storage, req) requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response) requireOneLogStatement(r, logger, `"failure" failureType:webhook authentication,msg:some webhook error`) }) - it("CreateSucceedsWithAnUnauthenticatedStatusWhenWebhookReturnsNilResponseWithAuthenticatedFalse", func() { - webhook := FakeToken{ - returnResponse: nil, - returnUnauthenticated: false, - } - storage := NewREST(&webhook, nil) - - response, err := callCreate(context.Background(), storage, validCredentialRequest()) - - requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response) - requireOneLogStatement(r, logger, `"success" userID:,idpAuthenticated:true,pinnipedAuthenticated:false`) - }) - - it("CreateSucceedsWithAnUnauthenticatedStatusWhenWebhookDoesNotReturnAnyUserInfo", func() { - webhook := FakeToken{ - returnResponse: &authenticator.Response{}, - returnUnauthenticated: false, - } - storage := NewREST(&webhook, nil) - - response, err := callCreate(context.Background(), storage, validCredentialRequest()) - - requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response) - requireOneLogStatement(r, logger, `"success" userID:,idpAuthenticated:true,pinnipedAuthenticated:false`) - }) - it("CreateSucceedsWithAnUnauthenticatedStatusWhenWebhookReturnsAnEmptyUsername", func() { - webhook := FakeToken{ - returnResponse: &authenticator.Response{ - User: &user.DefaultInfo{ - Name: "", - }, - }, - } - storage := NewREST(&webhook, nil) + req := validCredentialRequest() - response, err := callCreate(context.Background(), storage, validCredentialRequest()) + requestAuthenticator := credentialrequestmocks.NewMockTokenCredentialRequestAuthenticator(ctrl) + requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req). + Return(&user.DefaultInfo{Name: ""}, nil) + + storage := NewREST(requestAuthenticator, nil) + + response, err := callCreate(context.Background(), storage, req) requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response) - requireOneLogStatement(r, logger, `"success" userID:,idpAuthenticated:true,pinnipedAuthenticated:false`) - }) - - it("CreateDoesNotPassAdditionalContextInfoToTheWebhook", func() { - webhook := FakeToken{ - returnResponse: webhookSuccessResponse(), - } - storage := NewREST(&webhook, successfulIssuer(ctrl)) - ctx := context.WithValue(context.Background(), contextKey{}, "context-value") - - _, err := callCreate(ctx, storage, validCredentialRequest()) - - r.NoError(err) - r.Nil(webhook.calledWithContext.Value("context-key")) + requireOneLogStatement(r, logger, `"success" userID:,authenticated:false`) }) it("CreateFailsWhenGivenTheWrongInputType", func() { notACredentialRequest := runtime.Unknown{} - response, err := NewREST(&FakeToken{}, nil).Create( + response, err := NewREST(nil, nil).Create( genericapirequest.NewContext(), ¬ACredentialRequest, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}) - requireAPIError(t, response, err, apierrors.IsBadRequest, "not a CredentialRequest") - requireOneLogStatement(r, logger, `"failure" failureType:request validation,msg:not a CredentialRequest`) + requireAPIError(t, response, err, apierrors.IsBadRequest, "not a TokenCredentialRequest") + requireOneLogStatement(r, logger, `"failure" failureType:request validation,msg:not a TokenCredentialRequest`) }) it("CreateFailsWhenTokenValueIsEmptyInRequest", func() { - storage := NewREST(&FakeToken{}, nil) + storage := NewREST(nil, nil) response, err := callCreate(context.Background(), storage, credentialRequest(loginapi.TokenCredentialRequestSpec{ Token: "", })) @@ -326,7 +186,7 @@ func TestCreate(t *testing.T) { }) it("CreateFailsWhenValidationFails", func() { - storage := NewREST(&FakeToken{}, nil) + storage := NewREST(nil, nil) response, err := storage.Create( context.Background(), validCredentialRequest(), @@ -340,14 +200,16 @@ func TestCreate(t *testing.T) { }) it("CreateDoesNotAllowValidationFunctionToMutateRequest", func() { - webhook := FakeToken{ - returnResponse: webhookSuccessResponse(), - } - storage := NewREST(&webhook, successfulIssuer(ctrl)) - requestToken := "a token" + req := validCredentialRequest() + + requestAuthenticator := credentialrequestmocks.NewMockTokenCredentialRequestAuthenticator(ctrl) + requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req.DeepCopy()). + Return(&user.DefaultInfo{Name: "test-user"}, nil) + + storage := NewREST(requestAuthenticator, successfulIssuer(ctrl)) response, err := storage.Create( context.Background(), - validCredentialRequestWithToken(requestToken), + req, func(ctx context.Context, obj runtime.Object) error { credentialRequest, _ := obj.(*loginapi.TokenCredentialRequest) credentialRequest.Spec.Token = "foobaz" @@ -356,20 +218,21 @@ func TestCreate(t *testing.T) { &metav1.CreateOptions{}) r.NoError(err) r.NotEmpty(response) - r.Equal(requestToken, webhook.calledWithToken) // i.e. not called with foobaz }) it("CreateDoesNotAllowValidationFunctionToSeeTheActualRequestToken", func() { - webhook := FakeToken{ - returnResponse: webhookSuccessResponse(), - } + req := validCredentialRequest() - storage := NewREST(&webhook, successfulIssuer(ctrl)) + requestAuthenticator := credentialrequestmocks.NewMockTokenCredentialRequestAuthenticator(ctrl) + requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req.DeepCopy()). + Return(&user.DefaultInfo{Name: "test-user"}, nil) + + storage := NewREST(requestAuthenticator, successfulIssuer(ctrl)) validationFunctionWasCalled := false var validationFunctionSawTokenValue string response, err := storage.Create( context.Background(), - validCredentialRequest(), + req, func(ctx context.Context, obj runtime.Object) error { credentialRequest, _ := obj.(*loginapi.TokenCredentialRequest) validationFunctionWasCalled = true @@ -384,7 +247,7 @@ func TestCreate(t *testing.T) { }) it("CreateFailsWhenRequestOptionsDryRunIsNotEmpty", func() { - response, err := NewREST(&FakeToken{}, nil).Create( + response, err := NewREST(nil, nil).Create( genericapirequest.NewContext(), validCredentialRequest(), rest.ValidateAllObjectFunc, @@ -396,60 +259,6 @@ func TestCreate(t *testing.T) { `.pinniped.dev "request name" is invalid: dryRun: Unsupported value: []string{"some dry run flag"}`) requireOneLogStatement(r, logger, `"failure" failureType:request validation,msg:dryRun not supported`) }) - - it("CreateCancelsTheWebhookInvocationWhenTheCallToCreateIsCancelledItself", func() { - webhookStartedRunningNotificationChan := make(chan bool) - webhook := FakeToken{ - timeout: time.Second * 2, - webhookStartedRunningNotificationChan: webhookStartedRunningNotificationChan, - returnResponse: webhookSuccessResponse(), - } - storage := NewREST(&webhook, successfulIssuer(ctrl)) - ctx, cancel := context.WithCancel(context.Background()) - - c := make(chan bool) - go func() { - _, err := callCreate(ctx, storage, validCredentialRequest()) - c <- true - r.NoError(err) - }() - - r.False(webhook.cancelled) - r.False(webhook.reachedTimeout) - <-webhookStartedRunningNotificationChan // wait long enough to make sure that the webhook has started - cancel() // cancel the context that was passed to storage.Create() above - <-c // wait for the above call to storage.Create() to be finished - r.True(webhook.cancelled) - r.False(webhook.reachedTimeout) - r.Equal(context.Canceled, webhook.calledWithContext.Err()) // the inner context is cancelled - }) - - it("CreateAllowsTheWebhookInvocationToFinishWhenTheCallToCreateIsNotCancelledItself", func() { - webhookStartedRunningNotificationChan := make(chan bool) - webhook := FakeToken{ - timeout: 0, - webhookStartedRunningNotificationChan: webhookStartedRunningNotificationChan, - returnResponse: webhookSuccessResponse(), - } - storage := NewREST(&webhook, successfulIssuer(ctrl)) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - c := make(chan bool) - go func() { - _, err := callCreate(ctx, storage, validCredentialRequest()) - c <- true - r.NoError(err) - }() - - r.False(webhook.cancelled) - r.False(webhook.reachedTimeout) - <-webhookStartedRunningNotificationChan // wait long enough to make sure that the webhook has started - <-c // wait for the above call to storage.Create() to be finished - r.False(webhook.cancelled) - r.True(webhook.reachedTimeout) - r.Equal(context.Canceled, webhook.calledWithContext.Err()) // the inner context is cancelled (in this case by the "defer") - }) }, spec.Sequential()) } @@ -488,15 +297,6 @@ func credentialRequest(spec loginapi.TokenCredentialRequestSpec) *loginapi.Token } } -func webhookSuccessResponse() *authenticator.Response { - return &authenticator.Response{User: &user.DefaultInfo{ - Name: "some-user", - UID: "", - Groups: []string{}, - Extra: nil, - }} -} - func requireAPIError(t *testing.T, response runtime.Object, err error, expectedErrorTypeChecker func(err error) bool, expectedErrorMessage string) { t.Helper() require.Nil(t, response) @@ -518,7 +318,7 @@ func requireSuccessfulResponseWithAuthenticationFailureMessage(t *testing.T, err } func successfulIssuer(ctrl *gomock.Controller) CertIssuer { - issuer := mockcertissuer.NewMockCertIssuer(ctrl) + issuer := credentialrequestmocks.NewMockCertIssuer(ctrl) issuer.EXPECT(). IssuePEM(gomock.Any(), gomock.Any(), gomock.Any()). Return([]byte("test-cert"), []byte("test-key"), nil) diff --git a/internal/server/server.go b/internal/server/server.go index 1420182d..6839c424 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -12,7 +12,6 @@ import ( "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apiserver/pkg/authentication/authenticator" genericapiserver "k8s.io/apiserver/pkg/server" genericoptions "k8s.io/apiserver/pkg/server/options" "k8s.io/client-go/kubernetes" @@ -246,8 +245,8 @@ func getClusterCASigner( // Create a configuration for the aggregated API server. func getAggregatedAPIServerConfig( dynamicCertProvider provider.DynamicTLSServingCertProvider, - tokenAuthenticator authenticator.Token, - ca credentialrequest.CertIssuer, + authenticator credentialrequest.TokenCredentialRequestAuthenticator, + issuer credentialrequest.CertIssuer, startControllersPostStartHook func(context.Context), ) (*apiserver.Config, error) { recommendedOptions := genericoptions.NewRecommendedOptions( @@ -275,8 +274,8 @@ func getAggregatedAPIServerConfig( apiServerConfig := &apiserver.Config{ GenericConfig: serverConfig, ExtraConfig: apiserver.ExtraConfig{ - TokenAuthenticator: tokenAuthenticator, - Issuer: ca, + Authenticator: authenticator, + Issuer: issuer, StartControllersPostStartHook: startControllersPostStartHook, }, } From 541336b9978077264197894bd64c64ea37480965 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 21 Sep 2020 13:02:59 -0500 Subject: [PATCH 04/11] Fix docstring for exchange credential CLI. Signed-off-by: Matt Moyer --- cmd/pinniped/cmd/exchange_credential.go | 6 ++++++ cmd/pinniped/cmd/exchange_credential_test.go | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/cmd/pinniped/cmd/exchange_credential.go b/cmd/pinniped/cmd/exchange_credential.go index 1ad0470f..77679b8f 100644 --- a/cmd/pinniped/cmd/exchange_credential.go +++ b/cmd/pinniped/cmd/exchange_credential.go @@ -60,6 +60,12 @@ func newExchangeCredentialCmd(args []string, stdout, stderr io.Writer) *exchange Requires all of the following environment variables, which are typically set in the kubeconfig: - PINNIPED_TOKEN: the token to send to Pinniped for exchange + - PINNIPED_NAMESPACE: the namespace of the identity provider to authenticate + against + - PINNIPED_IDP_TYPE: the type of identity provider to authenticate + against (e.g., "webhook") + - PINNIPED_IDP_NAME: the name of the identity provider to authenticate + against - PINNIPED_CA_BUNDLE: the CA bundle to trust when calling Pinniped's HTTPS endpoint - PINNIPED_K8S_API_ENDPOINT: the URL for the Pinniped credential diff --git a/cmd/pinniped/cmd/exchange_credential_test.go b/cmd/pinniped/cmd/exchange_credential_test.go index 1d74f186..b074cfc0 100644 --- a/cmd/pinniped/cmd/exchange_credential_test.go +++ b/cmd/pinniped/cmd/exchange_credential_test.go @@ -43,6 +43,12 @@ var ( Requires all of the following environment variables, which are typically set in the kubeconfig: - PINNIPED_TOKEN: the token to send to Pinniped for exchange + - PINNIPED_NAMESPACE: the namespace of the identity provider to authenticate + against + - PINNIPED_IDP_TYPE: the type of identity provider to authenticate + against (e.g., "webhook") + - PINNIPED_IDP_NAME: the name of the identity provider to authenticate + against - PINNIPED_CA_BUNDLE: the CA bundle to trust when calling Pinniped's HTTPS endpoint - PINNIPED_K8S_API_ENDPOINT: the URL for the Pinniped credential From 381fd51e132cda458aadaee601218072de86f98f Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 21 Sep 2020 15:42:54 -0500 Subject: [PATCH 05/11] Refactor get_kubeconfig.go. Signed-off-by: Matt Moyer --- cmd/pinniped/cmd/get_kubeconfig.go | 174 ++---- cmd/pinniped/cmd/get_kubeconfig_test.go | 795 +++++++----------------- 2 files changed, 261 insertions(+), 708 deletions(-) diff --git a/cmd/pinniped/cmd/get_kubeconfig.go b/cmd/pinniped/cmd/get_kubeconfig.go index 2acb5282..7992f087 100644 --- a/cmd/pinniped/cmd/get_kubeconfig.go +++ b/cmd/pinniped/cmd/get_kubeconfig.go @@ -27,51 +27,40 @@ import ( "go.pinniped.dev/internal/here" ) -const ( - getKubeConfigCmdTokenFlagName = "token" - getKubeConfigCmdKubeconfigFlagName = "kubeconfig" - getKubeConfigCmdKubeconfigContextFlagName = "kubeconfig-context" - getKubeConfigCmdPinnipedNamespaceFlagName = "pinniped-namespace" -) - //nolint: gochecknoinits func init() { - rootCmd.AddCommand(newGetKubeConfigCmd(os.Args, os.Stdout, os.Stderr).cmd) + rootCmd.AddCommand(newGetKubeConfigCommand().Command()) +} + +type getKubeConfigFlags struct { + token string + kubeconfig string + contextOverride string + namespace string } type getKubeConfigCommand struct { - // runFunc is called by the cobra.Command.Run hook. It is included here for - // testability. - runFunc func( - stdout, stderr io.Writer, - token, kubeconfigPathOverride, currentContextOverride, pinnipedInstallationNamespace string, - ) - - // cmd is the cobra.Command for this CLI command. It is included here for - // testability. - cmd *cobra.Command + flags getKubeConfigFlags + // Test mocking points + getPathToSelf func() (string, error) + kubeClientCreator func(restConfig *rest.Config) (pinnipedclientset.Interface, error) } -func newGetKubeConfigCmd(args []string, stdout, stderr io.Writer) *getKubeConfigCommand { - c := &getKubeConfigCommand{ - runFunc: runGetKubeConfig, - } - - c.cmd = &cobra.Command{ - Run: func(cmd *cobra.Command, _ []string) { - token := cmd.Flag(getKubeConfigCmdTokenFlagName).Value.String() - kubeconfigPathOverride := cmd.Flag(getKubeConfigCmdKubeconfigFlagName).Value.String() - currentContextOverride := cmd.Flag(getKubeConfigCmdKubeconfigContextFlagName).Value.String() - pinnipedInstallationNamespace := cmd.Flag(getKubeConfigCmdPinnipedNamespaceFlagName).Value.String() - c.runFunc( - stdout, - stderr, - token, - kubeconfigPathOverride, - currentContextOverride, - pinnipedInstallationNamespace, - ) +func newGetKubeConfigCommand() *getKubeConfigCommand { + return &getKubeConfigCommand{ + flags: getKubeConfigFlags{ + namespace: "pinniped", }, + getPathToSelf: os.Executable, + kubeClientCreator: func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { + return pinnipedclientset.NewForConfig(restConfig) + }, + } +} + +func (c *getKubeConfigCommand) Command() *cobra.Command { + cmd := &cobra.Command{ + RunE: c.run, Args: cobra.NoArgs, // do not accept positional arguments for this command Use: "get-kubeconfig", Short: "Print a kubeconfig for authenticating into a cluster via Pinniped", @@ -93,94 +82,40 @@ func newGetKubeConfigCmd(args []string, stdout, stderr io.Writer) *getKubeConfig kubectl --kubeconfig $HOME/mycluster-kubeconfig get pods `), } - - c.cmd.SetArgs(args) - c.cmd.SetOut(stdout) - c.cmd.SetErr(stderr) - - c.cmd.Flags().StringP( - getKubeConfigCmdTokenFlagName, - "", - "", - "Credential to include in the resulting kubeconfig output (Required)", - ) - err := c.cmd.MarkFlagRequired(getKubeConfigCmdTokenFlagName) + cmd.Flags().StringVar(&c.flags.token, "token", "", "Credential to include in the resulting kubeconfig output (Required)") + err := cmd.MarkFlagRequired("token") if err != nil { panic(err) } - - c.cmd.Flags().StringP( - getKubeConfigCmdKubeconfigFlagName, - "", - "", - "Path to the kubeconfig file", - ) - - c.cmd.Flags().StringP( - getKubeConfigCmdKubeconfigContextFlagName, - "", - "", - "Kubeconfig context override", - ) - - c.cmd.Flags().StringP( - getKubeConfigCmdPinnipedNamespaceFlagName, - "", - "pinniped", - "Namespace in which Pinniped was installed", - ) - - return c + cmd.Flags().StringVar(&c.flags.kubeconfig, "kubeconfig", c.flags.kubeconfig, "Path to the kubeconfig file") + cmd.Flags().StringVar(&c.flags.contextOverride, "kubeconfig-context", c.flags.contextOverride, "Kubeconfig context override") + cmd.Flags().StringVar(&c.flags.namespace, "pinniped-namespace", c.flags.namespace, "Namespace in which Pinniped was installed") + return cmd } -func runGetKubeConfig( - stdout, stderr io.Writer, - token, kubeconfigPathOverride, currentContextOverride, pinnipedInstallationNamespace string, -) { - err := getKubeConfig( - stdout, - stderr, - token, - kubeconfigPathOverride, - currentContextOverride, - pinnipedInstallationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { - return pinnipedclientset.NewForConfig(restConfig) - }, - ) - - if err != nil { - _, _ = fmt.Fprintf(os.Stderr, "error: %s\n", err.Error()) - os.Exit(1) - } -} - -func getKubeConfig( - outputWriter io.Writer, - warningsWriter io.Writer, - token string, - kubeconfigPathOverride string, - currentContextNameOverride string, - pinnipedInstallationNamespace string, - kubeClientCreator func(restConfig *rest.Config) (pinnipedclientset.Interface, error), -) error { - if token == "" { - return constable.Error("--" + getKubeConfigCmdTokenFlagName + " flag value cannot be empty") - } - - fullPathToSelf, err := os.Executable() +func (c *getKubeConfigCommand) run(cmd *cobra.Command, args []string) error { + fullPathToSelf, err := c.getPathToSelf() if err != nil { return fmt.Errorf("could not find path to self: %w", err) } - clientConfig := newClientConfig(kubeconfigPathOverride, currentContextNameOverride) + clientConfig := newClientConfig(c.flags.kubeconfig, c.flags.contextOverride) currentKubeConfig, err := clientConfig.RawConfig() if err != nil { return err } - credentialIssuerConfig, err := fetchPinnipedCredentialIssuerConfig(clientConfig, kubeClientCreator, pinnipedInstallationNamespace) + restConfig, err := clientConfig.ClientConfig() + if err != nil { + return err + } + clientset, err := c.kubeClientCreator(restConfig) + if err != nil { + return err + } + + credentialIssuerConfig, err := fetchPinnipedCredentialIssuerConfig(clientset, c.flags.namespace) if err != nil { return err } @@ -189,19 +124,19 @@ func getKubeConfig( return constable.Error(`CredentialIssuerConfig "pinniped-config" was missing KubeConfigInfo`) } - v1Cluster, err := copyCurrentClusterFromExistingKubeConfig(currentKubeConfig, currentContextNameOverride) + v1Cluster, err := copyCurrentClusterFromExistingKubeConfig(currentKubeConfig, c.flags.contextOverride) if err != nil { return err } - err = issueWarningForNonMatchingServerOrCA(v1Cluster, credentialIssuerConfig, warningsWriter) + err = issueWarningForNonMatchingServerOrCA(v1Cluster, credentialIssuerConfig, cmd.ErrOrStderr()) if err != nil { return err } - config := newPinnipedKubeconfig(v1Cluster, fullPathToSelf, token, pinnipedInstallationNamespace) + config := newPinnipedKubeconfig(v1Cluster, fullPathToSelf, c.flags.token, c.flags.namespace) - err = writeConfigAsYAML(outputWriter, config) + err = writeConfigAsYAML(cmd.OutOrStdout(), config) if err != nil { return err } @@ -224,16 +159,7 @@ func issueWarningForNonMatchingServerOrCA(v1Cluster v1.Cluster, credentialIssuer return nil } -func fetchPinnipedCredentialIssuerConfig(clientConfig clientcmd.ClientConfig, kubeClientCreator func(restConfig *rest.Config) (pinnipedclientset.Interface, error), pinnipedInstallationNamespace string) (*configv1alpha1.CredentialIssuerConfig, error) { - restConfig, err := clientConfig.ClientConfig() - if err != nil { - return nil, err - } - clientset, err := kubeClientCreator(restConfig) - if err != nil { - return nil, err - } - +func fetchPinnipedCredentialIssuerConfig(clientset pinnipedclientset.Interface, pinnipedInstallationNamespace string) (*configv1alpha1.CredentialIssuerConfig, error) { ctx, cancelFunc := context.WithTimeout(context.Background(), time.Second*20) defer cancelFunc() diff --git a/cmd/pinniped/cmd/get_kubeconfig_test.go b/cmd/pinniped/cmd/get_kubeconfig_test.go index 817fbd4f..d8bb7232 100644 --- a/cmd/pinniped/cmd/get_kubeconfig_test.go +++ b/cmd/pinniped/cmd/get_kubeconfig_test.go @@ -7,16 +7,15 @@ import ( "bytes" "encoding/base64" "fmt" - "io" - "os" + "strings" "testing" - "github.com/sclevine/spec" - "github.com/sclevine/spec/report" + "github.com/spf13/cobra" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" + coretesting "k8s.io/client-go/testing" configv1alpha1 "go.pinniped.dev/generated/1.19/apis/config/v1alpha1" pinnipedclientset "go.pinniped.dev/generated/1.19/client/clientset/versioned" @@ -68,149 +67,60 @@ var ( ) func TestNewGetKubeConfigCmd(t *testing.T) { - spec.Run(t, "newGetKubeConfigCmd", func(t *testing.T, when spec.G, it spec.S) { - var r *require.Assertions - var stdout, stderr *bytes.Buffer + t.Parallel() + tests := []struct { + name string + args []string + wantError bool + wantStdout string + wantStderr string + }{ + { + name: "help flag passed", + args: []string{"--help"}, + wantStdout: knownGoodHelpForGetKubeConfig, + }, + { + name: "missing required flag", + args: []string{}, + wantError: true, + wantStdout: `Error: required flag(s) "token" not set` + "\n" + knownGoodUsageForGetKubeConfig, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cmd := newGetKubeConfigCommand().Command() + require.NotNil(t, cmd) - it.Before(func() { - r = require.New(t) - - stdout, stderr = bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{}) + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs(tt.args) + err := cmd.Execute() + if tt.wantError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + require.Equal(t, tt.wantStdout, stdout.String(), "unexpected stdout") + require.Equal(t, tt.wantStderr, stderr.String(), "unexpected stderr") }) - - it("passes all flags to runFunc", func() { - args := []string{ - "--token", "some-token", - "--kubeconfig", "some-kubeconfig", - "--kubeconfig-context", "some-kubeconfig-context", - "--pinniped-namespace", "some-pinniped-namespace", - } - c := newGetKubeConfigCmd(args, stdout, stderr) - - runFuncCalled := false - c.runFunc = func( - out, err io.Writer, - token, kubeconfigPathOverride, currentContextOverride, pinnipedInstallationNamespace string, - ) { - runFuncCalled = true - r.Equal("some-token", token) - r.Equal("some-kubeconfig", kubeconfigPathOverride) - r.Equal("some-kubeconfig-context", currentContextOverride) - r.Equal("some-pinniped-namespace", pinnipedInstallationNamespace) - } - - r.NoError(c.cmd.Execute()) - r.True(runFuncCalled) - r.Empty(stdout.String()) - r.Empty(stderr.String()) - }) - - it("requires the 'token' flag", func() { - args := []string{ - "--kubeconfig", "some-kubeconfig", - "--kubeconfig-context", "some-kubeconfig-context", - "--pinniped-namespace", "some-pinniped-namespace", - } - c := newGetKubeConfigCmd(args, stdout, stderr) - - runFuncCalled := false - c.runFunc = func( - out, err io.Writer, - token, kubeconfigPathOverride, currentContextOverride, pinnipedInstallationNamespace string, - ) { - runFuncCalled = true - } - - errorMessage := `required flag(s) "token" not set` - r.EqualError(c.cmd.Execute(), errorMessage) - r.False(runFuncCalled) - - output := "Error: " + errorMessage + "\n" + knownGoodUsageForGetKubeConfig - r.Equal(output, stdout.String()) - r.Empty(stderr.String()) - }) - - it("defaults the flags correctly", func() { - args := []string{ - "--token", "some-token", - } - c := newGetKubeConfigCmd(args, stdout, stderr) - - runFuncCalled := false - c.runFunc = func( - out, err io.Writer, - token, kubeconfigPathOverride, currentContextOverride, pinnipedInstallationNamespace string, - ) { - runFuncCalled = true - r.Equal("some-token", token) - r.Equal("", kubeconfigPathOverride) - r.Equal("", currentContextOverride) - r.Equal("pinniped", pinnipedInstallationNamespace) - } - - r.NoError(c.cmd.Execute()) - r.True(runFuncCalled) - r.Empty(stdout.String()) - r.Empty(stderr.String()) - }) - - it("fails when args are passed", func() { - args := []string{ - "--token", "some-token", - "some-arg", - } - c := newGetKubeConfigCmd(args, stdout, stderr) - - runFuncCalled := false - c.runFunc = func( - out, err io.Writer, - token, kubeconfigPathOverride, currentContextOverride, pinnipedInstallationNamespace string, - ) { - runFuncCalled = true - } - - errorMessage := `unknown command "some-arg" for "get-kubeconfig"` - r.EqualError(c.cmd.Execute(), errorMessage) - r.False(runFuncCalled) - - output := "Error: " + errorMessage + "\n" + knownGoodUsageForGetKubeConfig - r.Equal(output, stdout.String()) - r.Empty(stderr.String()) - }) - - it("prints a nice help message", func() { - args := []string{ - "--help", - } - c := newGetKubeConfigCmd(args, stdout, stderr) - - runFuncCalled := false - c.runFunc = func( - out, err io.Writer, - token, kubeconfigPathOverride, currentContextOverride, pinnipedInstallationNamespace string, - ) { - runFuncCalled = true - } - - r.NoError(c.cmd.Execute()) - r.False(runFuncCalled) - r.Equal(knownGoodHelpForGetKubeConfig, stdout.String()) - r.Empty(stderr.String()) - }) - }, spec.Parallel(), spec.Report(report.Terminal{})) + } } -func expectedKubeconfigYAML( - clusterCAData, - clusterServer, - command, - // nolint: unparam // Pass in the token even if it is always the same in practice - token, - pinnipedEndpoint, - pinnipedCABundle, - // nolint: unparam // Pass in the namespace even if it is always the same in practice - namespace string, -) string { +type expectedKubeconfigYAML struct { + clusterCAData string + clusterServer string + command string + token string + pinnipedEndpoint string + pinnipedCABundle string + namespace string +} + +func (e expectedKubeconfigYAML) String() string { return here.Docf(` apiVersion: v1 clusters: @@ -246,16 +156,10 @@ func expectedKubeconfigYAML( installHint: |- The Pinniped CLI is required to authenticate to the current cluster. For more information, please visit https://pinniped.dev - `, clusterCAData, clusterServer, command, pinnipedEndpoint, pinnipedCABundle, namespace, token) + `, e.clusterCAData, e.clusterServer, e.command, e.pinnipedEndpoint, e.pinnipedCABundle, e.namespace, e.token) } -func newCredentialIssuerConfig( - name, - //nolint: unparam // Pass in the namespace even if it is always the same in practice - namespace, - server, - certificateAuthorityData string, -) *configv1alpha1.CredentialIssuerConfig { +func newCredentialIssuerConfig(name, namespace, server, certificateAuthorityData string) *configv1alpha1.CredentialIssuerConfig { return &configv1alpha1.CredentialIssuerConfig{ TypeMeta: metav1.TypeMeta{ Kind: "CredentialIssuerConfig", @@ -274,439 +178,162 @@ func newCredentialIssuerConfig( } } -func TestGetKubeConfig(t *testing.T) { - spec.Run(t, "cmd.getKubeConfig", func(t *testing.T, when spec.G, it spec.S) { - var r *require.Assertions - var outputBuffer *bytes.Buffer - var warningsBuffer *bytes.Buffer - var fullPathToSelf string - var pinnipedClient *pinnipedfake.Clientset - const installationNamespace = "some-namespace" +func TestRun(t *testing.T) { + t.Parallel() + tests := []struct { + name string + mocks func(*getKubeConfigCommand) + wantError string + wantStdout string + wantStderr string + }{ + { + name: "failure to get path to self", + mocks: func(cmd *getKubeConfigCommand) { + cmd.getPathToSelf = func() (string, error) { + return "", fmt.Errorf("some error getting path to self") + } + }, + wantError: "could not find path to self: some error getting path to self", + }, + { + name: "kubeconfig does not exist", + mocks: func(cmd *getKubeConfigCommand) { + cmd.flags.kubeconfig = "./testdata/does-not-exist.yaml" + }, + wantError: "stat ./testdata/does-not-exist.yaml: no such file or directory", + }, + { + name: "fail to get client", + mocks: func(cmd *getKubeConfigCommand) { + cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { + return nil, fmt.Errorf("some error configuring clientset") + } + }, + wantError: "some error configuring clientset", + }, + { + name: "fail to get CredentialIssuerConfigs", + mocks: func(cmd *getKubeConfigCommand) { + clientset := pinnipedfake.NewSimpleClientset() + clientset.PrependReactor("*", "*", func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("some error getting CredentialIssuerConfigs") + }) + cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { + return clientset, nil + } + }, + wantError: "some error getting CredentialIssuerConfigs", + }, + { + name: "zero CredentialIssuerConfigs found", + mocks: func(cmd *getKubeConfigCommand) { + cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { + return pinnipedfake.NewSimpleClientset( + newCredentialIssuerConfig("pinniped-config-1", "not-the-test-namespace", "", ""), + ), nil + } + }, + wantError: `No CredentialIssuerConfig was found in namespace "test-namespace". Is Pinniped installed on this cluster in namespace "test-namespace"?`, + }, + { + name: "multiple CredentialIssuerConfigs found", + mocks: func(cmd *getKubeConfigCommand) { + cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { + return pinnipedfake.NewSimpleClientset( + newCredentialIssuerConfig("pinniped-config-1", "test-namespace", "", ""), + newCredentialIssuerConfig("pinniped-config-2", "test-namespace", "", ""), + ), nil + } + }, + wantError: `More than one CredentialIssuerConfig was found in namespace "test-namespace"`, + }, + { + name: "CredentialIssuerConfig missing KubeConfigInfo", + mocks: func(cmd *getKubeConfigCommand) { + cic := newCredentialIssuerConfig("pinniped-config", "test-namespace", "", "") + cic.Status.KubeConfigInfo = nil + cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { + return pinnipedfake.NewSimpleClientset(cic), nil + } + }, + wantError: `CredentialIssuerConfig "pinniped-config" was missing KubeConfigInfo`, + }, + { + name: "KubeConfigInfo has invalid base64", + mocks: func(cmd *getKubeConfigCommand) { + cic := newCredentialIssuerConfig("pinniped-config", "test-namespace", "https://example.com", "") + cic.Status.KubeConfigInfo.CertificateAuthorityData = "invalid-base64-test-ca" + cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { + return pinnipedfake.NewSimpleClientset(cic), nil + } + }, + wantError: `illegal base64 data at input byte 7`, + }, + { + name: "success using remote CA data", + mocks: func(cmd *getKubeConfigCommand) { + cic := newCredentialIssuerConfig("pinniped-config", "test-namespace", "https://fake-server-url-value", "fake-certificate-authority-data-value") + cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { + return pinnipedfake.NewSimpleClientset(cic), nil + } + }, + wantStdout: expectedKubeconfigYAML{ + clusterCAData: "ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ==", + clusterServer: "https://fake-server-url-value", + command: "/path/to/pinniped", + token: "test-token", + pinnipedEndpoint: "https://fake-server-url-value", + pinnipedCABundle: "fake-certificate-authority-data-value", + namespace: "test-namespace", + }.String(), + }, + { + name: "success using local CA data", + mocks: func(cmd *getKubeConfigCommand) { + cic := newCredentialIssuerConfig("pinniped-config", "test-namespace", "https://example.com", "test-ca") + cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { + return pinnipedfake.NewSimpleClientset(cic), nil + } + }, + wantStderr: `WARNING: Server and certificate authority did not match between local kubeconfig and Pinniped's CredentialIssuerConfig on the cluster. Using local kubeconfig values.`, + wantStdout: expectedKubeconfigYAML{ + clusterCAData: "ZmFrZS1jZXJ0aWZpY2F0ZS1hdXRob3JpdHktZGF0YS12YWx1ZQ==", + clusterServer: "https://fake-server-url-value", + command: "/path/to/pinniped", + token: "test-token", + pinnipedEndpoint: "https://fake-server-url-value", + pinnipedCABundle: "fake-certificate-authority-data-value", + namespace: "test-namespace", + }.String(), + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() - it.Before(func() { - r = require.New(t) + // Start with a default getKubeConfigCommand, set some defaults, then apply any mocks. + c := newGetKubeConfigCommand() + c.flags.token = "test-token" + c.flags.namespace = "test-namespace" + c.getPathToSelf = func() (string, error) { return "/path/to/pinniped", nil } + c.flags.kubeconfig = "./testdata/kubeconfig.yaml" + tt.mocks(c) - outputBuffer = new(bytes.Buffer) - warningsBuffer = new(bytes.Buffer) - - var err error - fullPathToSelf, err = os.Executable() - r.NoError(err) - - pinnipedClient = pinnipedfake.NewSimpleClientset() + cmd := &cobra.Command{} + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs([]string{}) + err := c.run(cmd, []string{}) + if tt.wantError != "" { + require.EqualError(t, err, tt.wantError) + } else { + require.NoError(t, err) + } + require.Equal(t, strings.TrimSpace(tt.wantStdout), strings.TrimSpace(stdout.String()), "unexpected stdout") + require.Equal(t, strings.TrimSpace(tt.wantStderr), strings.TrimSpace(stderr.String()), "unexpected stderr") }) - - when("the CredentialIssuerConfig is found on the cluster with a configuration that matches the existing kubeconfig", func() { - it.Before(func() { - r.NoError(pinnipedClient.Tracker().Add( - newCredentialIssuerConfig( - "some-cic-name", - installationNamespace, - "https://fake-server-url-value", - "fake-certificate-authority-data-value", - ), - )) - }) - - it("writes the kubeconfig to the given writer", func() { - kubeClientCreatorFuncWasCalled := false - err := getKubeConfig(outputBuffer, - warningsBuffer, - "some-token", - "./testdata/kubeconfig.yaml", - "", - installationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { - kubeClientCreatorFuncWasCalled = true - r.Equal("https://fake-server-url-value", restConfig.Host) - r.Equal("fake-certificate-authority-data-value", string(restConfig.CAData)) - return pinnipedClient, nil - }, - ) - r.NoError(err) - r.True(kubeClientCreatorFuncWasCalled) - - r.Empty(warningsBuffer.String()) - r.Equal(expectedKubeconfigYAML( - base64.StdEncoding.EncodeToString([]byte("fake-certificate-authority-data-value")), - "https://fake-server-url-value", - fullPathToSelf, - "some-token", - "https://fake-server-url-value", - "fake-certificate-authority-data-value", - installationNamespace, - ), outputBuffer.String()) - }) - - when("the currentContextOverride is used to specify a context other than the default context", func() { - it.Before(func() { - // update the Server and CertificateAuthorityData to make them match the other kubeconfig context - r.NoError(pinnipedClient.Tracker().Update( - schema.GroupVersionResource{ - Group: configv1alpha1.GroupName, - Version: configv1alpha1.SchemeGroupVersion.Version, - Resource: "credentialissuerconfigs", - }, - newCredentialIssuerConfig( - "some-cic-name", - installationNamespace, - "https://some-other-fake-server-url-value", - "some-other-fake-certificate-authority-data-value", - ), - installationNamespace, - )) - }) - - when("that context exists", func() { - it("writes the kubeconfig to the given writer using the specified context", func() { - kubeClientCreatorFuncWasCalled := false - err := getKubeConfig(outputBuffer, - warningsBuffer, - "some-token", - "./testdata/kubeconfig.yaml", - "some-other-context", - installationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { - kubeClientCreatorFuncWasCalled = true - r.Equal("https://some-other-fake-server-url-value", restConfig.Host) - r.Equal("some-other-fake-certificate-authority-data-value", string(restConfig.CAData)) - return pinnipedClient, nil - }, - ) - r.NoError(err) - r.True(kubeClientCreatorFuncWasCalled) - - r.Empty(warningsBuffer.String()) - r.Equal(expectedKubeconfigYAML( - base64.StdEncoding.EncodeToString([]byte("some-other-fake-certificate-authority-data-value")), - "https://some-other-fake-server-url-value", - fullPathToSelf, - "some-token", - "https://some-other-fake-server-url-value", - "some-other-fake-certificate-authority-data-value", - installationNamespace, - ), outputBuffer.String()) - }) - }) - - when("that context does not exist the in the current kubeconfig", func() { - it("returns an error", func() { - err := getKubeConfig(outputBuffer, - warningsBuffer, - "some-token", - "./testdata/kubeconfig.yaml", - "this-context-name-does-not-exist-in-kubeconfig.yaml", - installationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { return pinnipedClient, nil }, - ) - r.EqualError(err, `context "this-context-name-does-not-exist-in-kubeconfig.yaml" does not exist`) - r.Empty(warningsBuffer.String()) - r.Empty(outputBuffer.String()) - }) - }) - }) - - when("the token passed in is empty", func() { - it("returns an error", func() { - err := getKubeConfig(outputBuffer, - warningsBuffer, - "", - "./testdata/kubeconfig.yaml", - "", - installationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { return pinnipedClient, nil }, - ) - r.EqualError(err, "--token flag value cannot be empty") - r.Empty(warningsBuffer.String()) - r.Empty(outputBuffer.String()) - }) - }) - - when("the kubeconfig path passed refers to a file that does not exist", func() { - it("returns an error", func() { - err := getKubeConfig(outputBuffer, - warningsBuffer, - "some-token", - "./testdata/this-file-does-not-exist.yaml", - "", - installationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { return pinnipedClient, nil }, - ) - r.EqualError(err, "stat ./testdata/this-file-does-not-exist.yaml: no such file or directory") - r.Empty(warningsBuffer.String()) - r.Empty(outputBuffer.String()) - }) - }) - - when("the kubeconfig path parameter is empty", func() { - it.Before(func() { - // Note that this is technically polluting other parallel tests in this file, but other tests - // are always specifying the kubeconfigPathOverride parameter, so they're not actually looking - // at the value of this environment variable. - r.NoError(os.Setenv("KUBECONFIG", "./testdata/kubeconfig.yaml")) - }) - - it.After(func() { - r.NoError(os.Unsetenv("KUBECONFIG")) - }) - - it("falls back to using the KUBECONFIG env var to find the kubeconfig file", func() { - kubeClientCreatorFuncWasCalled := false - err := getKubeConfig(outputBuffer, - warningsBuffer, - "some-token", - "", - "", - installationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { - kubeClientCreatorFuncWasCalled = true - r.Equal("https://fake-server-url-value", restConfig.Host) - r.Equal("fake-certificate-authority-data-value", string(restConfig.CAData)) - return pinnipedClient, nil - }, - ) - r.NoError(err) - r.True(kubeClientCreatorFuncWasCalled) - - r.Empty(warningsBuffer.String()) - r.Equal(expectedKubeconfigYAML( - base64.StdEncoding.EncodeToString([]byte("fake-certificate-authority-data-value")), - "https://fake-server-url-value", - fullPathToSelf, - "some-token", - "https://fake-server-url-value", - "fake-certificate-authority-data-value", - installationNamespace, - ), outputBuffer.String()) - }) - }) - - when("the wrong pinniped namespace is passed in", func() { - it("returns an error", func() { - kubeClientCreatorFuncWasCalled := false - err := getKubeConfig(outputBuffer, - warningsBuffer, - "some-token", - "./testdata/kubeconfig.yaml", - "", - "this-is-the-wrong-namespace", - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { - kubeClientCreatorFuncWasCalled = true - r.Equal("https://fake-server-url-value", restConfig.Host) - r.Equal("fake-certificate-authority-data-value", string(restConfig.CAData)) - return pinnipedClient, nil - }, - ) - r.EqualError(err, `No CredentialIssuerConfig was found in namespace "this-is-the-wrong-namespace". Is Pinniped installed on this cluster in namespace "this-is-the-wrong-namespace"?`) - r.True(kubeClientCreatorFuncWasCalled) - }) - }) - - when("there is more than one CredentialIssuerConfig is found on the cluster", func() { - it.Before(func() { - r.NoError(pinnipedClient.Tracker().Add( - newCredentialIssuerConfig( - "another-cic-name", - installationNamespace, - "https://fake-server-url-value", - "fake-certificate-authority-data-value", - ), - )) - }) - - it("returns an error", func() { - kubeClientCreatorFuncWasCalled := false - err := getKubeConfig(outputBuffer, - warningsBuffer, - "some-token", - "./testdata/kubeconfig.yaml", - "", - installationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { - kubeClientCreatorFuncWasCalled = true - r.Equal("https://fake-server-url-value", restConfig.Host) - r.Equal("fake-certificate-authority-data-value", string(restConfig.CAData)) - return pinnipedClient, nil - }, - ) - r.EqualError(err, `More than one CredentialIssuerConfig was found in namespace "some-namespace"`) - r.True(kubeClientCreatorFuncWasCalled) - }) - }) - }) - - when("the CredentialIssuerConfig is found on the cluster with a configuration that does not match the existing kubeconfig", func() { - when("the Server doesn't match", func() { - it.Before(func() { - r.NoError(pinnipedClient.Tracker().Add( - newCredentialIssuerConfig( - "some-cic-name", - installationNamespace, - "non-matching-pinniped-server-url", - "fake-certificate-authority-data-value", - ), - )) - }) - - it("writes the kubeconfig to the given writer using the values found in the local kubeconfig and issues a warning", func() { - kubeClientCreatorFuncWasCalled := false - err := getKubeConfig(outputBuffer, - warningsBuffer, - "some-token", - "./testdata/kubeconfig.yaml", - "", - installationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { - kubeClientCreatorFuncWasCalled = true - r.Equal("https://fake-server-url-value", restConfig.Host) - r.Equal("fake-certificate-authority-data-value", string(restConfig.CAData)) - return pinnipedClient, nil - }, - ) - r.NoError(err) - r.True(kubeClientCreatorFuncWasCalled) - - r.Equal( - "WARNING: Server and certificate authority did not match between local kubeconfig and Pinniped's CredentialIssuerConfig on the cluster. Using local kubeconfig values.\n", - warningsBuffer.String(), - ) - r.Equal(expectedKubeconfigYAML( - base64.StdEncoding.EncodeToString([]byte("fake-certificate-authority-data-value")), - "https://fake-server-url-value", - fullPathToSelf, - "some-token", - "https://fake-server-url-value", - "fake-certificate-authority-data-value", - installationNamespace, - ), outputBuffer.String()) - }) - }) - - when("the CA doesn't match", func() { - it.Before(func() { - r.NoError(pinnipedClient.Tracker().Add( - newCredentialIssuerConfig( - "some-cic-name", - installationNamespace, - "https://fake-server-url-value", - "non-matching-certificate-authority-data-value", - ), - )) - }) - - it("writes the kubeconfig to the given writer using the values found in the local kubeconfig and issues a warning", func() { - kubeClientCreatorFuncWasCalled := false - err := getKubeConfig(outputBuffer, - warningsBuffer, - "some-token", - "./testdata/kubeconfig.yaml", - "", - installationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { - kubeClientCreatorFuncWasCalled = true - r.Equal("https://fake-server-url-value", restConfig.Host) - r.Equal("fake-certificate-authority-data-value", string(restConfig.CAData)) - return pinnipedClient, nil - }, - ) - r.NoError(err) - r.True(kubeClientCreatorFuncWasCalled) - - r.Equal( - "WARNING: Server and certificate authority did not match between local kubeconfig and Pinniped's CredentialIssuerConfig on the cluster. Using local kubeconfig values.\n", - warningsBuffer.String(), - ) - r.Equal(expectedKubeconfigYAML( - base64.StdEncoding.EncodeToString([]byte("fake-certificate-authority-data-value")), - "https://fake-server-url-value", - fullPathToSelf, - "some-token", - "https://fake-server-url-value", - "fake-certificate-authority-data-value", - installationNamespace, - ), outputBuffer.String()) - }) - }) - }) - - when("the CredentialIssuerConfig is found on the cluster with an empty KubeConfigInfo", func() { - it.Before(func() { - r.NoError(pinnipedClient.Tracker().Add( - &configv1alpha1.CredentialIssuerConfig{ - TypeMeta: metav1.TypeMeta{ - Kind: "CredentialIssuerConfig", - APIVersion: configv1alpha1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "pinniped-config", - Namespace: installationNamespace, - }, - Status: configv1alpha1.CredentialIssuerConfigStatus{}, - }, - )) - }) - - it("returns an error", func() { - kubeClientCreatorFuncWasCalled := false - err := getKubeConfig(outputBuffer, - warningsBuffer, - "some-token", - "./testdata/kubeconfig.yaml", - "", - installationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { - kubeClientCreatorFuncWasCalled = true - r.Equal("https://fake-server-url-value", restConfig.Host) - r.Equal("fake-certificate-authority-data-value", string(restConfig.CAData)) - return pinnipedClient, nil - }, - ) - r.True(kubeClientCreatorFuncWasCalled) - r.EqualError(err, `CredentialIssuerConfig "pinniped-config" was missing KubeConfigInfo`) - r.Empty(warningsBuffer.String()) - r.Empty(outputBuffer.String()) - }) - }) - - when("the CredentialIssuerConfig does not exist on the cluster", func() { - it("returns an error", func() { - kubeClientCreatorFuncWasCalled := false - err := getKubeConfig(outputBuffer, - warningsBuffer, - "some-token", - "./testdata/kubeconfig.yaml", - "", - installationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { - kubeClientCreatorFuncWasCalled = true - r.Equal("https://fake-server-url-value", restConfig.Host) - r.Equal("fake-certificate-authority-data-value", string(restConfig.CAData)) - return pinnipedClient, nil - }, - ) - r.True(kubeClientCreatorFuncWasCalled) - r.EqualError(err, `No CredentialIssuerConfig was found in namespace "some-namespace". Is Pinniped installed on this cluster in namespace "some-namespace"?`) - r.Empty(warningsBuffer.String()) - r.Empty(outputBuffer.String()) - }) - }) - - when("there is an error while getting the CredentialIssuerConfig from the cluster", func() { - it("returns an error", func() { - err := getKubeConfig(outputBuffer, - warningsBuffer, - "some-token", - "./testdata/kubeconfig.yaml", - "", - installationNamespace, - func(restConfig *rest.Config) (pinnipedclientset.Interface, error) { - return nil, fmt.Errorf("some error getting CredentialIssuerConfig") - }, - ) - r.EqualError(err, "some error getting CredentialIssuerConfig") - r.Empty(warningsBuffer.String()) - r.Empty(outputBuffer.String()) - }) - }) - }, spec.Parallel(), spec.Report(report.Terminal{})) + } } From 481308215d8b7ff72cad9572c442615ee6805dc5 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 21 Sep 2020 17:31:07 -0500 Subject: [PATCH 06/11] Pass namespace properly in client.ExchangeToken. Signed-off-by: Matt Moyer --- internal/client/client.go | 3 +++ internal/client/client_test.go | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/client/client.go b/internal/client/client.go index 06831cc8..25445a74 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -30,6 +30,9 @@ func ExchangeToken(ctx context.Context, namespace string, idp corev1.TypedLocalO } resp, err := client.LoginV1alpha1().TokenCredentialRequests(namespace).Create(ctx, &v1alpha1.TokenCredentialRequest{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + }, Spec: v1alpha1.TokenCredentialRequestSpec{ Token: token, IdentityProvider: idp, diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 482f588a..fd4a564a 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -100,7 +100,8 @@ func TestExchangeToken(t *testing.T) { "kind": "TokenCredentialRequest", "apiVersion": "login.pinniped.dev/v1alpha1", "metadata": { - "creationTimestamp": null + "creationTimestamp": null, + "namespace": "test-namespace" }, "spec": { "token": "test-token", From 07f0181fa3b92ed77e2ff23c69e7a9bb9ff5d190 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 21 Sep 2020 17:41:30 -0500 Subject: [PATCH 07/11] Add IDP selection to get-kubeconfig command. Signed-off-by: Matt Moyer --- cmd/pinniped/cmd/get_kubeconfig.go | 63 +++++++++++++++++++++- cmd/pinniped/cmd/get_kubeconfig_test.go | 70 +++++++++++++++++++++++-- 2 files changed, 127 insertions(+), 6 deletions(-) diff --git a/cmd/pinniped/cmd/get_kubeconfig.go b/cmd/pinniped/cmd/get_kubeconfig.go index 7992f087..0660c556 100644 --- a/cmd/pinniped/cmd/get_kubeconfig.go +++ b/cmd/pinniped/cmd/get_kubeconfig.go @@ -37,6 +37,8 @@ type getKubeConfigFlags struct { kubeconfig string contextOverride string namespace string + idpName string + idpType string } type getKubeConfigCommand struct { @@ -90,6 +92,8 @@ func (c *getKubeConfigCommand) Command() *cobra.Command { cmd.Flags().StringVar(&c.flags.kubeconfig, "kubeconfig", c.flags.kubeconfig, "Path to the kubeconfig file") cmd.Flags().StringVar(&c.flags.contextOverride, "kubeconfig-context", c.flags.contextOverride, "Kubeconfig context override") cmd.Flags().StringVar(&c.flags.namespace, "pinniped-namespace", c.flags.namespace, "Namespace in which Pinniped was installed") + cmd.Flags().StringVar(&c.flags.idpType, "idp-type", c.flags.idpType, "Identity provider type (e.g., 'webhook')") + cmd.Flags().StringVar(&c.flags.idpName, "idp-name", c.flags.idpType, "Identity provider name") return cmd } @@ -115,6 +119,14 @@ func (c *getKubeConfigCommand) run(cmd *cobra.Command, args []string) error { return err } + idpType, idpName := c.flags.idpType, c.flags.idpName + if idpType == "" || idpName == "" { + idpType, idpName, err = getDefaultIDP(clientset, c.flags.namespace) + if err != nil { + return err + } + } + credentialIssuerConfig, err := fetchPinnipedCredentialIssuerConfig(clientset, c.flags.namespace) if err != nil { return err @@ -134,7 +146,7 @@ func (c *getKubeConfigCommand) run(cmd *cobra.Command, args []string) error { return err } - config := newPinnipedKubeconfig(v1Cluster, fullPathToSelf, c.flags.token, c.flags.namespace) + config := newPinnipedKubeconfig(v1Cluster, fullPathToSelf, c.flags.token, c.flags.namespace, idpType, idpName) err = writeConfigAsYAML(cmd.OutOrStdout(), config) if err != nil { @@ -159,6 +171,45 @@ func issueWarningForNonMatchingServerOrCA(v1Cluster v1.Cluster, credentialIssuer return nil } +type noIDPError struct{ Namespace string } + +func (e noIDPError) Error() string { + return fmt.Sprintf(`no identity providers were found in namespace %q`, e.Namespace) +} + +type indeterminateIDPError struct{ Namespace string } + +func (e indeterminateIDPError) Error() string { + return fmt.Sprintf( + `multiple identity providers were found in namespace %q, so --pinniped-idp-name/--pinniped-idp-type must be specified`, + e.Namespace, + ) +} + +func getDefaultIDP(clientset pinnipedclientset.Interface, namespace string) (string, string, error) { + ctx, cancelFunc := context.WithTimeout(context.Background(), time.Second*20) + defer cancelFunc() + + webhooks, err := clientset.IDPV1alpha1().WebhookIdentityProviders(namespace).List(ctx, metav1.ListOptions{}) + if err != nil { + return "", "", err + } + + type ref struct{ idpType, idpName string } + idps := make([]ref, 0, len(webhooks.Items)) + for _, webhook := range webhooks.Items { + idps = append(idps, ref{idpType: "webhook", idpName: webhook.Name}) + } + + if len(idps) == 0 { + return "", "", noIDPError{namespace} + } + if len(idps) > 1 { + return "", "", indeterminateIDPError{namespace} + } + return idps[0].idpType, idps[0].idpName, nil +} + func fetchPinnipedCredentialIssuerConfig(clientset pinnipedclientset.Interface, pinnipedInstallationNamespace string) (*configv1alpha1.CredentialIssuerConfig, error) { ctx, cancelFunc := context.WithTimeout(context.Background(), time.Second*20) defer cancelFunc() @@ -229,7 +280,7 @@ func copyCurrentClusterFromExistingKubeConfig(currentKubeConfig clientcmdapi.Con return v1Cluster, nil } -func newPinnipedKubeconfig(v1Cluster v1.Cluster, fullPathToSelf string, token string, namespace string) v1.Config { +func newPinnipedKubeconfig(v1Cluster v1.Cluster, fullPathToSelf string, token string, namespace string, idpType string, idpName string) v1.Config { clusterName := "pinniped-cluster" userName := "pinniped-user" @@ -275,6 +326,14 @@ func newPinnipedKubeconfig(v1Cluster v1.Cluster, fullPathToSelf string, token st Name: "PINNIPED_TOKEN", Value: token, }, + { + Name: "PINNIPED_IDP_TYPE", + Value: idpType, + }, + { + Name: "PINNIPED_IDP_NAME", + Value: idpName, + }, }, APIVersion: clientauthenticationv1beta1.SchemeGroupVersion.String(), InstallHint: "The Pinniped CLI is required to authenticate to the current cluster.\n" + diff --git a/cmd/pinniped/cmd/get_kubeconfig_test.go b/cmd/pinniped/cmd/get_kubeconfig_test.go index d8bb7232..7834e1b4 100644 --- a/cmd/pinniped/cmd/get_kubeconfig_test.go +++ b/cmd/pinniped/cmd/get_kubeconfig_test.go @@ -18,6 +18,7 @@ import ( coretesting "k8s.io/client-go/testing" configv1alpha1 "go.pinniped.dev/generated/1.19/apis/config/v1alpha1" + idpv1alpha "go.pinniped.dev/generated/1.19/apis/idp/v1alpha1" pinnipedclientset "go.pinniped.dev/generated/1.19/client/clientset/versioned" pinnipedfake "go.pinniped.dev/generated/1.19/client/clientset/versioned/fake" "go.pinniped.dev/internal/here" @@ -30,6 +31,8 @@ var ( Flags: -h, --help help for get-kubeconfig + --idp-name string Identity provider name + --idp-type string Identity provider type (e.g., 'webhook') --kubeconfig string Path to the kubeconfig file --kubeconfig-context string Kubeconfig context override --pinniped-namespace string Namespace in which Pinniped was installed (default "pinniped") @@ -59,6 +62,8 @@ var ( Flags: -h, --help help for get-kubeconfig + --idp-name string Identity provider name + --idp-type string Identity provider type (e.g., 'webhook') --kubeconfig string Path to the kubeconfig file --kubeconfig-context string Kubeconfig context override --pinniped-namespace string Namespace in which Pinniped was installed (default "pinniped") @@ -118,6 +123,8 @@ type expectedKubeconfigYAML struct { pinnipedEndpoint string pinnipedCABundle string namespace string + idpType string + idpName string } func (e expectedKubeconfigYAML) String() string { @@ -153,10 +160,14 @@ func (e expectedKubeconfigYAML) String() string { value: %s - name: PINNIPED_TOKEN value: %s + - name: PINNIPED_IDP_TYPE + value: %s + - name: PINNIPED_IDP_NAME + value: %s installHint: |- The Pinniped CLI is required to authenticate to the current cluster. For more information, please visit https://pinniped.dev - `, e.clusterCAData, e.clusterServer, e.command, e.pinnipedEndpoint, e.pinnipedCABundle, e.namespace, e.token) + `, e.clusterCAData, e.clusterServer, e.command, e.pinnipedEndpoint, e.pinnipedCABundle, e.namespace, e.token, e.idpType, e.idpName) } func newCredentialIssuerConfig(name, namespace, server, certificateAuthorityData string) *configv1alpha1.CredentialIssuerConfig { @@ -212,6 +223,46 @@ func TestRun(t *testing.T) { }, wantError: "some error configuring clientset", }, + { + name: "fail to get IDPs", + mocks: func(cmd *getKubeConfigCommand) { + cmd.flags.idpName = "" + cmd.flags.idpType = "" + clientset := pinnipedfake.NewSimpleClientset() + clientset.PrependReactor("*", "*", func(_ coretesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("some error getting IDPs") + }) + cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { + return clientset, nil + } + }, + wantError: "some error getting IDPs", + }, + { + name: "zero IDPs", + mocks: func(cmd *getKubeConfigCommand) { + cmd.flags.idpName = "" + cmd.flags.idpType = "" + cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { + return pinnipedfake.NewSimpleClientset(), nil + } + }, + wantError: `no identity providers were found in namespace "test-namespace"`, + }, + { + name: "multiple IDPs", + mocks: func(cmd *getKubeConfigCommand) { + cmd.flags.idpName = "" + cmd.flags.idpType = "" + cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { + return pinnipedfake.NewSimpleClientset( + &idpv1alpha.WebhookIdentityProvider{ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "webhook-one"}}, + &idpv1alpha.WebhookIdentityProvider{ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "webhook-two"}}, + ), nil + } + }, + wantError: `multiple identity providers were found in namespace "test-namespace", so --pinniped-idp-name/--pinniped-idp-type must be specified`, + }, { name: "fail to get CredentialIssuerConfigs", mocks: func(cmd *getKubeConfigCommand) { @@ -286,14 +337,21 @@ func TestRun(t *testing.T) { pinnipedEndpoint: "https://fake-server-url-value", pinnipedCABundle: "fake-certificate-authority-data-value", namespace: "test-namespace", + idpType: "test-idp-type", + idpName: "test-idp-name", }.String(), }, { - name: "success using local CA data", + name: "success using local CA data and discovered IDP", mocks: func(cmd *getKubeConfigCommand) { - cic := newCredentialIssuerConfig("pinniped-config", "test-namespace", "https://example.com", "test-ca") + cmd.flags.idpName = "" + cmd.flags.idpType = "" + cmd.kubeClientCreator = func(_ *rest.Config) (pinnipedclientset.Interface, error) { - return pinnipedfake.NewSimpleClientset(cic), nil + return pinnipedfake.NewSimpleClientset( + &idpv1alpha.WebhookIdentityProvider{ObjectMeta: metav1.ObjectMeta{Namespace: "test-namespace", Name: "discovered-idp"}}, + newCredentialIssuerConfig("pinniped-config", "test-namespace", "https://example.com", "test-ca"), + ), nil } }, wantStderr: `WARNING: Server and certificate authority did not match between local kubeconfig and Pinniped's CredentialIssuerConfig on the cluster. Using local kubeconfig values.`, @@ -305,6 +363,8 @@ func TestRun(t *testing.T) { pinnipedEndpoint: "https://fake-server-url-value", pinnipedCABundle: "fake-certificate-authority-data-value", namespace: "test-namespace", + idpType: "webhook", + idpName: "discovered-idp", }.String(), }, } @@ -317,6 +377,8 @@ func TestRun(t *testing.T) { c := newGetKubeConfigCommand() c.flags.token = "test-token" c.flags.namespace = "test-namespace" + c.flags.idpName = "test-idp-name" + c.flags.idpType = "test-idp-type" c.getPathToSelf = func() (string, error) { return "/path/to/pinniped", nil } c.flags.kubeconfig = "./testdata/kubeconfig.yaml" tt.mocks(c) From 81f2362543f31c38269f1b2c0335a8cb8e2e2610 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 21 Sep 2020 17:42:27 -0500 Subject: [PATCH 08/11] Remove fallback support for implicitly choosing an IDP in TokenCredentialRequest. Signed-off-by: Matt Moyer --- .../identityprovider/idpcache/cache.go | 19 ---------- .../identityprovider/idpcache/cache_test.go | 35 ------------------- 2 files changed, 54 deletions(-) diff --git a/internal/controller/identityprovider/idpcache/cache.go b/internal/controller/identityprovider/idpcache/cache.go index ddbe63c7..d9ffa614 100644 --- a/internal/controller/identityprovider/idpcache/cache.go +++ b/internal/controller/identityprovider/idpcache/cache.go @@ -18,12 +18,6 @@ import ( var ( // ErrNoSuchIDP is returned by Cache.AuthenticateTokenCredentialRequest() when the requested IDP is not configured. ErrNoSuchIDP = fmt.Errorf("no such identity provider") - - // ErrNoIDPs is returned by Cache.AuthenticateTokenCredentialRequest() when there are no IDPs configured. - ErrNoIDPs = fmt.Errorf("no identity providers are loaded") - - // ErrIndeterminateIDP is returned by Cache.AuthenticateTokenCredentialRequest() when the correct IDP cannot be determined. - ErrIndeterminateIDP = fmt.Errorf("could not uniquely match against an identity provider") ) // Cache implements the authenticator.Token interface by multiplexing across a dynamic set of identity providers @@ -88,19 +82,6 @@ func (c *Cache) AuthenticateTokenCredentialRequest(ctx context.Context, req *log key.APIGroup = *req.Spec.IdentityProvider.APIGroup } - // If the IDP is unspecified (legacy requests), choose the single loaded IDP or fail if there is not exactly - // one IDP configured. - if key.Name == "" || key.Kind == "" || key.APIGroup == "" { - keys := c.Keys() - if len(keys) == 0 { - return nil, ErrNoIDPs - } - if len(keys) > 1 { - return nil, ErrIndeterminateIDP - } - key = keys[0] - } - val := c.Get(key) if val == nil { return nil, ErrNoSuchIDP diff --git a/internal/controller/identityprovider/idpcache/cache_test.go b/internal/controller/identityprovider/idpcache/cache_test.go index 33d156da..e400ff0e 100644 --- a/internal/controller/identityprovider/idpcache/cache_test.go +++ b/internal/controller/identityprovider/idpcache/cache_test.go @@ -51,41 +51,6 @@ func TestCache(t *testing.T) { func TestAuthenticateTokenCredentialRequest(t *testing.T) { t.Parallel() - t.Run("missing IDP selector", func(t *testing.T) { - t.Run("no IDPs", func(t *testing.T) { - c := New() - res, err := c.AuthenticateTokenCredentialRequest(context.Background(), &loginapi.TokenCredentialRequest{}) - require.EqualError(t, err, "no identity providers are loaded") - require.Nil(t, res) - }) - - t.Run("multiple IDPs", func(t *testing.T) { - c := New() - c.Store(Key{Name: "idp-one"}, nil) - c.Store(Key{Name: "idp-two"}, nil) - res, err := c.AuthenticateTokenCredentialRequest(context.Background(), &loginapi.TokenCredentialRequest{}) - require.EqualError(t, err, "could not uniquely match against an identity provider") - require.Nil(t, res) - }) - - t.Run("single IDP", func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - c := New() - mockToken := mocktokenauthenticator.NewMockToken(ctrl) - mockToken.EXPECT().AuthenticateToken(gomock.Any(), "test-token"). - Return(&authenticator.Response{User: &user.DefaultInfo{Name: "test-user"}}, true, nil) - c.Store(Key{Name: "idp-one"}, mockToken) - - res, err := c.AuthenticateTokenCredentialRequest(context.Background(), &loginapi.TokenCredentialRequest{ - Spec: loginapi.TokenCredentialRequestSpec{Token: "test-token"}, - }) - require.NoError(t, err) - require.Equal(t, "test-user", res.GetName()) - }) - }) - validRequest := loginapi.TokenCredentialRequest{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test-namespace", From 9beb3855b5e32f9aca31f134d617581e0fad8c21 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 21 Sep 2020 19:55:04 -0500 Subject: [PATCH 09/11] Create webhooks per-test and explicitly in `demo.md` instead of with ytt in `./deploy`. Signed-off-by: Matt Moyer --- deploy/values.yaml | 4 -- deploy/webhook.yaml | 16 -------- doc/demo.md | 22 +++++++--- hack/prepare-for-integration-tests.sh | 4 +- test/integration/cli_test.go | 17 +++++--- test/integration/client_test.go | 9 +---- test/integration/credentialrequest_test.go | 34 ++++++++++------ test/library/client.go | 47 ++++++++++++++++++++++ 8 files changed, 101 insertions(+), 52 deletions(-) delete mode 100644 deploy/webhook.yaml diff --git a/deploy/values.yaml b/deploy/values.yaml index bae0dabd..b579a212 100644 --- a/deploy/values.yaml +++ b/deploy/values.yaml @@ -21,10 +21,6 @@ image_tag: latest #! Optional. image_pull_dockerconfigjson: #! e.g. {"auths":{"https://registry.example.com":{"username":"USERNAME","password":"PASSWORD","auth":"BASE64_ENCODED_USERNAME_COLON_PASSWORD"}}} -#! Configure a webhook identity provider. -webhook_url: #! e.g., https://example.com -webhook_ca_bundle: #! Must be a base64 encoded PEM certificate. e.g., LS0tLS1CRUdJTiBDRVJUSUZJQ0F... - #! Pinniped will try to guess the right K8s API URL for sharing that information with potential clients. #! This settings allows the guess to be overridden. #! Optional. diff --git a/deploy/webhook.yaml b/deploy/webhook.yaml deleted file mode 100644 index 4dd08e08..00000000 --- a/deploy/webhook.yaml +++ /dev/null @@ -1,16 +0,0 @@ -#! Copyright 2020 the Pinniped contributors. All Rights Reserved. -#! SPDX-License-Identifier: Apache-2.0 - -#@ load("@ytt:data", "data") - -apiVersion: idp.pinniped.dev/v1alpha1 -kind: WebhookIdentityProvider -metadata: - name: #@ data.values.app_name + "-webhook" - namespace: #@ data.values.namespace - labels: - app: #@ data.values.app_name -spec: - endpoint: #@ data.values.webhook_url - tls: - certificateAuthorityData: #@ data.values.webhook_ca_bundle diff --git a/doc/demo.md b/doc/demo.md index 7fb2feb2..74152243 100644 --- a/doc/demo.md +++ b/doc/demo.md @@ -83,12 +83,24 @@ ```bash cd /tmp/pinniped/deploy - ytt --file . \ - --data-value "webhook_url=https://local-user-authenticator.local-user-authenticator.svc/authenticate" \ - --data-value "webhook_ca_bundle=$(cat /tmp/local-user-authenticator-ca-base64-encoded)" \ - | kapp deploy --yes --app pinniped --diff-changes --file - + ytt --file . | kapp deploy --yes --app pinniped --diff-changes --file - ``` +1. Create a `WebhookIdentityProvider` object to configure Pinniped to authenticate using `local-user-authenticator` + + ```bash + cat < /tmp/pinniped-kubeconfig + pinniped get-kubeconfig --token "pinny-the-seal:password123" --idp-type webhook --idp-name local-user-authenticator > /tmp/pinniped-kubeconfig ``` Note that the above command will print a warning to the screen. You can ignore this warning. diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index e22fc2fd..8340f2e4 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -191,8 +191,6 @@ ytt --file . \ --data-value "namespace=$namespace" \ --data-value "image_repo=$registry_repo" \ --data-value "image_tag=$tag" \ - --data-value "webhook_url=$webhook_url" \ - --data-value "webhook_ca_bundle=$webhook_ca_bundle" \ --data-value "discovery_url=$discovery_url" >"$manifest" kapp deploy --yes --app "$app_name" --diff-changes --file "$manifest" @@ -212,6 +210,8 @@ export PINNIPED_APP_NAME=${app_name} export PINNIPED_TEST_USER_USERNAME=${test_username} export PINNIPED_TEST_USER_GROUPS=${test_groups} export PINNIPED_TEST_USER_TOKEN=${test_username}:${test_password} +export PINNIPED_TEST_WEBHOOK_ENDPOINT=${webhook_url} +export PINNIPED_TEST_WEBHOOK_CA_BUNDLE=${webhook_ca_bundle} read -r -d '' PINNIPED_CLUSTER_CAPABILITY_YAML << PINNIPED_CLUSTER_CAPABILITY_YAML_EOF || true ${pinniped_cluster_capability_file_content} diff --git a/test/integration/cli_test.go b/test/integration/cli_test.go index 43eec1a4..738e6bef 100644 --- a/test/integration/cli_test.go +++ b/test/integration/cli_test.go @@ -27,6 +27,12 @@ func TestCLI(t *testing.T) { strings.ReplaceAll(library.GetEnv(t, "PINNIPED_TEST_USER_GROUPS"), " ", ""), ",", ) + // Create a test webhook configuration to use with the CLI. + ctx, cancelFunc := context.WithTimeout(context.Background(), 30*time.Second) + defer cancelFunc() + + idp := library.CreateTestWebhookIDP(ctx, t) + // Remove all Pinniped environment variables for the remainder of this test // because some of their names clash with the env vars expected by our // kubectl exec plugin. We would like this test to prove that the exec @@ -56,14 +62,11 @@ func TestCLI(t *testing.T) { defer cleanupFunc() // Run pinniped CLI to get kubeconfig. - kubeConfigYAML := runPinnipedCLI(t, pinnipedExe, token, namespaceName) - - adminClient := library.NewClientset(t) - ctx, cancelFunc := context.WithTimeout(context.Background(), time.Second*3) - defer cancelFunc() + kubeConfigYAML := runPinnipedCLI(t, pinnipedExe, token, namespaceName, "webhook", idp.Name) // In addition to the client-go based testing below, also try the kubeconfig // with kubectl to validate that it works. + adminClient := library.NewClientset(t) t.Run( "access as user with kubectl", accessAsUserWithKubectlTest(ctx, adminClient, kubeConfigYAML, testUsername, namespaceName), @@ -108,7 +111,7 @@ func buildPinnipedCLI(t *testing.T) (string, func()) { } } -func runPinnipedCLI(t *testing.T, pinnipedExe, token, namespaceName string) string { +func runPinnipedCLI(t *testing.T, pinnipedExe, token, namespaceName, idpType, idpName string) string { t.Helper() output, err := exec.Command( @@ -116,6 +119,8 @@ func runPinnipedCLI(t *testing.T, pinnipedExe, token, namespaceName string) stri "get-kubeconfig", "--token", token, "--pinniped-namespace", namespaceName, + "--idp-type", idpType, + "--idp-name", idpName, ).CombinedOutput() require.NoError(t, err, string(output)) diff --git a/test/integration/client_test.go b/test/integration/client_test.go index 1c451e2d..1482e278 100644 --- a/test/integration/client_test.go +++ b/test/integration/client_test.go @@ -10,9 +10,7 @@ import ( "time" "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" - idpv1alpha1 "go.pinniped.dev/generated/1.19/apis/idp/v1alpha1" "go.pinniped.dev/internal/client" "go.pinniped.dev/internal/here" "go.pinniped.dev/test/library" @@ -63,6 +61,8 @@ func TestClient(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() + idp := library.CreateTestWebhookIDP(ctx, t) + // Use an invalid certificate/key to validate that the ServerVersion API fails like we assume. invalidClient := library.NewClientsetWithCertAndKey(t, testCert, testKey) _, err := invalidClient.Discovery().ServerVersion() @@ -71,11 +71,6 @@ func TestClient(t *testing.T) { // Using the CA bundle and host from the current (admin) kubeconfig, do the token exchange. clientConfig := library.NewClientConfig(t) - idp := corev1.TypedLocalObjectReference{ - APIGroup: &idpv1alpha1.SchemeGroupVersion.Group, - Kind: "WebhookIdentityProvider", - Name: "pinniped-webhook", - } resp, err := client.ExchangeToken(ctx, namespace, idp, token, string(clientConfig.CAData), clientConfig.Host) require.NoError(t, err) require.NotNil(t, resp.Status.ExpirationTimestamp) diff --git a/test/integration/credentialrequest_test.go b/test/integration/credentialrequest_test.go index 8a3a5751..da2512bd 100644 --- a/test/integration/credentialrequest_test.go +++ b/test/integration/credentialrequest_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -26,8 +27,12 @@ func TestSuccessfulCredentialRequest(t *testing.T) { expectedTestUserGroups := strings.Split( strings.ReplaceAll(library.GetEnv(t, "PINNIPED_TEST_USER_GROUPS"), " ", ""), ",", ) + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() - response, err := makeRequest(t, validCredentialRequestSpecWithRealToken(t)) + testWebhook := library.CreateTestWebhookIDP(ctx, t) + + response, err := makeRequest(ctx, t, validCredentialRequestSpecWithRealToken(t, testWebhook)) require.NoError(t, err) require.NotNil(t, response.Status.Credential) @@ -41,9 +46,6 @@ func TestSuccessfulCredentialRequest(t *testing.T) { require.NotNil(t, response.Status.Credential.ExpirationTimestamp) require.InDelta(t, time.Until(response.Status.Credential.ExpirationTimestamp.Time), 1*time.Hour, float64(3*time.Minute)) - ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) - defer cancel() - // Create a client using the admin kubeconfig. adminClient := library.NewClientset(t) @@ -71,7 +73,7 @@ func TestFailedCredentialRequestWhenTheRequestIsValidButTheTokenDoesNotAuthentic library.SkipUnlessIntegration(t) library.SkipUnlessClusterHasCapability(t, library.ClusterSigningKeyIsAvailable) - response, err := makeRequest(t, v1alpha1.TokenCredentialRequestSpec{Token: "not a good token"}) + response, err := makeRequest(context.Background(), t, v1alpha1.TokenCredentialRequestSpec{Token: "not a good token"}) require.NoError(t, err) @@ -84,7 +86,7 @@ func TestCredentialRequest_ShouldFailWhenRequestDoesNotIncludeToken(t *testing.T library.SkipUnlessIntegration(t) library.SkipUnlessClusterHasCapability(t, library.ClusterSigningKeyIsAvailable) - response, err := makeRequest(t, v1alpha1.TokenCredentialRequestSpec{Token: ""}) + response, err := makeRequest(context.Background(), t, v1alpha1.TokenCredentialRequestSpec{Token: ""}) require.Error(t, err) statusError, isStatus := err.(*errors.StatusError) @@ -104,7 +106,12 @@ func TestCredentialRequest_OtherwiseValidRequestWithRealTokenShouldFailWhenTheCl library.SkipUnlessIntegration(t) library.SkipWhenClusterHasCapability(t, library.ClusterSigningKeyIsAvailable) - response, err := makeRequest(t, validCredentialRequestSpecWithRealToken(t)) + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + testWebhook := library.CreateTestWebhookIDP(ctx, t) + + response, err := makeRequest(ctx, t, validCredentialRequestSpecWithRealToken(t, testWebhook)) require.NoError(t, err) @@ -113,24 +120,27 @@ func TestCredentialRequest_OtherwiseValidRequestWithRealTokenShouldFailWhenTheCl require.Equal(t, stringPtr("authentication failed"), response.Status.Message) } -func makeRequest(t *testing.T, spec v1alpha1.TokenCredentialRequestSpec) (*v1alpha1.TokenCredentialRequest, error) { +func makeRequest(ctx context.Context, t *testing.T, spec v1alpha1.TokenCredentialRequestSpec) (*v1alpha1.TokenCredentialRequest, error) { t.Helper() client := library.NewAnonymousPinnipedClientset(t) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() ns := library.GetEnv(t, "PINNIPED_NAMESPACE") return client.LoginV1alpha1().TokenCredentialRequests(ns).Create(ctx, &v1alpha1.TokenCredentialRequest{ TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{}, + ObjectMeta: metav1.ObjectMeta{Namespace: ns}, Spec: spec, }, metav1.CreateOptions{}) } -func validCredentialRequestSpecWithRealToken(t *testing.T) v1alpha1.TokenCredentialRequestSpec { - return v1alpha1.TokenCredentialRequestSpec{Token: library.GetEnv(t, "PINNIPED_TEST_USER_TOKEN")} +func validCredentialRequestSpecWithRealToken(t *testing.T, idp corev1.TypedLocalObjectReference) v1alpha1.TokenCredentialRequestSpec { + return v1alpha1.TokenCredentialRequestSpec{ + Token: library.GetEnv(t, "PINNIPED_TEST_USER_TOKEN"), + IdentityProvider: idp, + } } func stringPtr(s string) *string { diff --git a/test/library/client.go b/test/library/client.go index 8249c647..49599ba8 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -4,17 +4,22 @@ package library import ( + "context" "io/ioutil" "os" "testing" + "time" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" aggregatorclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset" + idpv1alpha1 "go.pinniped.dev/generated/1.19/apis/idp/v1alpha1" pinnipedclientset "go.pinniped.dev/generated/1.19/client/clientset/versioned" // Import to initialize client auth plugins - the kubeconfig that we use for @@ -136,3 +141,45 @@ func newAnonymousClientRestConfigWithCertAndKeyAdded(t *testing.T, clientCertifi config.KeyData = []byte(clientKeyData) return config } + +// CreateTestWebhookIDP creates and returns a test WebhookIdentityProvider in $PINNIPED_NAMESPACE, which will be +// automatically deleted at the end of the current test's lifetime. It returns a corev1.TypedLocalObjectReference which +// descibes the test IDP within the test namespace. +func CreateTestWebhookIDP(ctx context.Context, t *testing.T) corev1.TypedLocalObjectReference { + t.Helper() + + namespace := GetEnv(t, "PINNIPED_NAMESPACE") + endpoint := GetEnv(t, "PINNIPED_TEST_WEBHOOK_ENDPOINT") + caBundle := GetEnv(t, "PINNIPED_TEST_WEBHOOK_CA_BUNDLE") + client := NewPinnipedClientset(t) + webhooks := client.IDPV1alpha1().WebhookIdentityProviders(namespace) + + createContext, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + idp, err := webhooks.Create(createContext, &idpv1alpha1.WebhookIdentityProvider{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "test-webhook-", + Labels: map[string]string{"pinniped.dev/test": t.Name()}, + }, + Spec: idpv1alpha1.WebhookIdentityProviderSpec{ + Endpoint: endpoint, + TLS: &idpv1alpha1.TLSSpec{CertificateAuthorityData: caBundle}, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err, "could not create test WebhookIdentityProvider") + t.Logf("created test WebhookIdentityProvider %s/%s", idp.Namespace, idp.Name) + + t.Cleanup(func() { + t.Logf("cleaning up test WebhookIdentityProvider %s/%s", idp.Namespace, idp.Name) + deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + err := webhooks.Delete(deleteCtx, idp.Name, metav1.DeleteOptions{}) + require.NoErrorf(t, err, "could not cleanup test WebhookIdentityProvider %s/%s", idp.Namespace, idp.Name) + }) + + return corev1.TypedLocalObjectReference{ + APIGroup: &idpv1alpha1.SchemeGroupVersion.Group, + Kind: "WebhookIdentityProvider", + Name: idp.Name, + } +} From 16ef2baf8a36afddfa45f4f9d560c9593f40eccf Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 22 Sep 2020 09:50:34 -0500 Subject: [PATCH 10/11] Sort idpcache keys to make things as deterministic as possible. Signed-off-by: Matt Moyer --- .../identityprovider/idpcache/cache.go | 9 +++++++++ .../identityprovider/idpcache/cache_test.go | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/internal/controller/identityprovider/idpcache/cache.go b/internal/controller/identityprovider/idpcache/cache.go index d9ffa614..cea71f14 100644 --- a/internal/controller/identityprovider/idpcache/cache.go +++ b/internal/controller/identityprovider/idpcache/cache.go @@ -7,6 +7,7 @@ package idpcache import ( "context" "fmt" + "sort" "sync" "k8s.io/apiserver/pkg/authentication/authenticator" @@ -68,6 +69,14 @@ func (c *Cache) Keys() []Key { result = append(result, key.(Key)) return true }) + + // Sort the results for consistency. + sort.Slice(result, func(i, j int) bool { + return result[i].APIGroup < result[j].APIGroup || + result[i].Kind < result[j].Kind || + result[i].Namespace < result[j].Namespace || + result[i].Name < result[j].Name + }) return result } diff --git a/internal/controller/identityprovider/idpcache/cache_test.go b/internal/controller/identityprovider/idpcache/cache_test.go index e400ff0e..83c49313 100644 --- a/internal/controller/identityprovider/idpcache/cache_test.go +++ b/internal/controller/identityprovider/idpcache/cache_test.go @@ -6,6 +6,7 @@ package idpcache import ( "context" "fmt" + "math/rand" "testing" "time" @@ -46,6 +47,24 @@ func TestCache(t *testing.T) { cache.Delete(key) } require.Zero(t, len(cache.Keys())) + + // Fill the cache back up with a fixed set of keys, but inserted in shuffled order. + keysInExpectedOrder := []Key{ + {APIGroup: "a", Kind: "a", Namespace: "a", Name: "a"}, + {APIGroup: "b", Kind: "a", Namespace: "a", Name: "a"}, + {APIGroup: "b", Kind: "b", Namespace: "a", Name: "a"}, + {APIGroup: "b", Kind: "b", Namespace: "b", Name: "a"}, + {APIGroup: "b", Kind: "b", Namespace: "b", Name: "b"}, + } + for tries := 0; tries < 10; tries++ { + cache := New() + for _, i := range rand.Perm(len(keysInExpectedOrder)) { + cache.Store(keysInExpectedOrder[i], nil) + } + + // Expect that they come back out in sorted order. + require.Equal(t, keysInExpectedOrder, cache.Keys()) + } } func TestAuthenticateTokenCredentialRequest(t *testing.T) { From e574a99c5e432ba09c48e2ce4b660983843b5e68 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 22 Sep 2020 10:02:32 -0500 Subject: [PATCH 11/11] Add an integration test that tries to use a non-existent IDP. Signed-off-by: Matt Moyer --- test/integration/credentialrequest_test.go | 32 +++++++++++++++++----- test/library/client.go | 1 + 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/test/integration/credentialrequest_test.go b/test/integration/credentialrequest_test.go index da2512bd..d27f652e 100644 --- a/test/integration/credentialrequest_test.go +++ b/test/integration/credentialrequest_test.go @@ -16,10 +16,28 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "go.pinniped.dev/generated/1.19/apis/login/v1alpha1" + idpv1alpha1 "go.pinniped.dev/generated/1.19/apis/idp/v1alpha1" + loginv1alpha1 "go.pinniped.dev/generated/1.19/apis/login/v1alpha1" "go.pinniped.dev/test/library" ) +func TestUnsuccessfulCredentialRequest(t *testing.T) { + library.SkipUnlessIntegration(t) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + response, err := makeRequest(ctx, t, validCredentialRequestSpecWithRealToken(t, corev1.TypedLocalObjectReference{ + APIGroup: &idpv1alpha1.SchemeGroupVersion.Group, + Kind: "WebhookIdentityProvider", + Name: "some-webhook-that-does-not-exist", + })) + require.NoError(t, err) + require.Nil(t, response.Status.Credential) + require.NotNil(t, response.Status.Message) + require.Equal(t, "authentication failed", *response.Status.Message) +} + func TestSuccessfulCredentialRequest(t *testing.T) { library.SkipUnlessIntegration(t) library.SkipUnlessClusterHasCapability(t, library.ClusterSigningKeyIsAvailable) @@ -73,7 +91,7 @@ func TestFailedCredentialRequestWhenTheRequestIsValidButTheTokenDoesNotAuthentic library.SkipUnlessIntegration(t) library.SkipUnlessClusterHasCapability(t, library.ClusterSigningKeyIsAvailable) - response, err := makeRequest(context.Background(), t, v1alpha1.TokenCredentialRequestSpec{Token: "not a good token"}) + response, err := makeRequest(context.Background(), t, loginv1alpha1.TokenCredentialRequestSpec{Token: "not a good token"}) require.NoError(t, err) @@ -86,7 +104,7 @@ func TestCredentialRequest_ShouldFailWhenRequestDoesNotIncludeToken(t *testing.T library.SkipUnlessIntegration(t) library.SkipUnlessClusterHasCapability(t, library.ClusterSigningKeyIsAvailable) - response, err := makeRequest(context.Background(), t, v1alpha1.TokenCredentialRequestSpec{Token: ""}) + response, err := makeRequest(context.Background(), t, loginv1alpha1.TokenCredentialRequestSpec{Token: ""}) require.Error(t, err) statusError, isStatus := err.(*errors.StatusError) @@ -120,7 +138,7 @@ func TestCredentialRequest_OtherwiseValidRequestWithRealTokenShouldFailWhenTheCl require.Equal(t, stringPtr("authentication failed"), response.Status.Message) } -func makeRequest(ctx context.Context, t *testing.T, spec v1alpha1.TokenCredentialRequestSpec) (*v1alpha1.TokenCredentialRequest, error) { +func makeRequest(ctx context.Context, t *testing.T, spec loginv1alpha1.TokenCredentialRequestSpec) (*loginv1alpha1.TokenCredentialRequest, error) { t.Helper() client := library.NewAnonymousPinnipedClientset(t) @@ -129,15 +147,15 @@ func makeRequest(ctx context.Context, t *testing.T, spec v1alpha1.TokenCredentia defer cancel() ns := library.GetEnv(t, "PINNIPED_NAMESPACE") - return client.LoginV1alpha1().TokenCredentialRequests(ns).Create(ctx, &v1alpha1.TokenCredentialRequest{ + return client.LoginV1alpha1().TokenCredentialRequests(ns).Create(ctx, &loginv1alpha1.TokenCredentialRequest{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{Namespace: ns}, Spec: spec, }, metav1.CreateOptions{}) } -func validCredentialRequestSpecWithRealToken(t *testing.T, idp corev1.TypedLocalObjectReference) v1alpha1.TokenCredentialRequestSpec { - return v1alpha1.TokenCredentialRequestSpec{ +func validCredentialRequestSpecWithRealToken(t *testing.T, idp corev1.TypedLocalObjectReference) loginv1alpha1.TokenCredentialRequestSpec { + return loginv1alpha1.TokenCredentialRequestSpec{ Token: library.GetEnv(t, "PINNIPED_TEST_USER_TOKEN"), IdentityProvider: idp, } diff --git a/test/library/client.go b/test/library/client.go index 49599ba8..ec14c8cd 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -170,6 +170,7 @@ func CreateTestWebhookIDP(ctx context.Context, t *testing.T) corev1.TypedLocalOb t.Logf("created test WebhookIdentityProvider %s/%s", idp.Namespace, idp.Name) t.Cleanup(func() { + t.Helper() t.Logf("cleaning up test WebhookIdentityProvider %s/%s", idp.Namespace, idp.Name) deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel()