Merge pull request #215 from mattmoyer/fix-upstream-oidc-provider

Fix some issues in the UpstreamOIDCProvider CRD and controller

apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go.tmpl

@@ -16,7 +16,7 @@ const (
 	// PhaseReady is the phase for an UpstreamOIDCProvider resource in a healthy state.
 	PhaseReady UpstreamOIDCProviderPhase = "Ready"
 
-	// PhaseErorr is the phase for an UpstreamOIDCProvider in an unhealthy state.
+	// PhaseError is the phase for an UpstreamOIDCProvider in an unhealthy state.
 	PhaseError UpstreamOIDCProviderPhase = "Error"
 )
 
@@ -40,6 +40,7 @@ type UpstreamOIDCProviderStatus struct {
 type OIDCAuthorizationConfig struct {
 	// AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization
 	// request flow with an OIDC identity provider. By default only the "openid" scope will be requested.
+	// +optional
 	AdditionalScopes []string `json:"additionalScopes"`
 }
 
@@ -47,10 +48,12 @@ type OIDCAuthorizationConfig struct {
 type OIDCClaims struct {
 	// Groups provides the name of the token claim that will be used to ascertain the groups to which
 	// an identity belongs.
+	// +optional
 	Groups string `json:"groups"`
 
 	// Username provides the name of the token claim that will be used to ascertain an identity's
 	// username.
+	// +optional
 	Username string `json:"username"`
 }
 
@@ -74,10 +77,12 @@ type UpstreamOIDCProviderSpec struct {
 
 	// AuthorizationConfig holds information about how to form the OAuth2 authorization request
 	// parameters to be used with this OIDC identity provider.
+	// +optional
 	AuthorizationConfig OIDCAuthorizationConfig `json:"authorizationConfig"`
 
 	// Claims provides the names of token claims that will be used when inspecting an identity from
 	// this OIDC identity provider.
+	// +optional
 	Claims OIDCClaims `json:"claims"`
 
 	// OIDCClient contains OIDC client information to be used used with this OIDC identity

deploy/supervisor/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml

@@ -64,8 +64,6 @@ spec:
                     items:
                       type: string
                     type: array
-                required:
-                - additionalScopes
                 type: object
               claims:
                 description: Claims provides the names of token claims that will be
@@ -79,9 +77,6 @@ spec:
                     description: Username provides the name of the token claim that
                       will be used to ascertain an identity's username.
                     type: string
-                required:
-                - groups
-                - username
                 type: object
               client:
                 description: OIDCClient contains OIDC client information to be used
@@ -104,8 +99,6 @@ spec:
                 pattern: ^https://
                 type: string
             required:
-            - authorizationConfig
-            - claims
             - client
             - issuer
             type: object

generated/1.17/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go

@@ -16,7 +16,7 @@ const (
 	// PhaseReady is the phase for an UpstreamOIDCProvider resource in a healthy state.
 	PhaseReady UpstreamOIDCProviderPhase = "Ready"
 
-	// PhaseErorr is the phase for an UpstreamOIDCProvider in an unhealthy state.
+	// PhaseError is the phase for an UpstreamOIDCProvider in an unhealthy state.
 	PhaseError UpstreamOIDCProviderPhase = "Error"
 )
 
@@ -40,6 +40,7 @@ type UpstreamOIDCProviderStatus struct {
 type OIDCAuthorizationConfig struct {
 	// AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization
 	// request flow with an OIDC identity provider. By default only the "openid" scope will be requested.
+	// +optional
 	AdditionalScopes []string `json:"additionalScopes"`
 }
 
@@ -47,10 +48,12 @@ type OIDCAuthorizationConfig struct {
 type OIDCClaims struct {
 	// Groups provides the name of the token claim that will be used to ascertain the groups to which
 	// an identity belongs.
+	// +optional
 	Groups string `json:"groups"`
 
 	// Username provides the name of the token claim that will be used to ascertain an identity's
 	// username.
+	// +optional
 	Username string `json:"username"`
 }
 
@@ -74,10 +77,12 @@ type UpstreamOIDCProviderSpec struct {
 
 	// AuthorizationConfig holds information about how to form the OAuth2 authorization request
 	// parameters to be used with this OIDC identity provider.
+	// +optional
 	AuthorizationConfig OIDCAuthorizationConfig `json:"authorizationConfig"`
 
 	// Claims provides the names of token claims that will be used when inspecting an identity from
 	// this OIDC identity provider.
+	// +optional
 	Claims OIDCClaims `json:"claims"`
 
 	// OIDCClient contains OIDC client information to be used used with this OIDC identity

generated/1.17/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml

@@ -64,8 +64,6 @@ spec:
                     items:
                       type: string
                     type: array
-                required:
-                - additionalScopes
                 type: object
               claims:
                 description: Claims provides the names of token claims that will be
@@ -79,9 +77,6 @@ spec:
                     description: Username provides the name of the token claim that
                       will be used to ascertain an identity's username.
                     type: string
-                required:
-                - groups
-                - username
                 type: object
               client:
                 description: OIDCClient contains OIDC client information to be used
@@ -104,8 +99,6 @@ spec:
                 pattern: ^https://
                 type: string
             required:
-            - authorizationConfig
-            - claims
             - client
             - issuer
             type: object

generated/1.18/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go

@@ -16,7 +16,7 @@ const (
 	// PhaseReady is the phase for an UpstreamOIDCProvider resource in a healthy state.
 	PhaseReady UpstreamOIDCProviderPhase = "Ready"
 
-	// PhaseErorr is the phase for an UpstreamOIDCProvider in an unhealthy state.
+	// PhaseError is the phase for an UpstreamOIDCProvider in an unhealthy state.
 	PhaseError UpstreamOIDCProviderPhase = "Error"
 )
 
@@ -40,6 +40,7 @@ type UpstreamOIDCProviderStatus struct {
 type OIDCAuthorizationConfig struct {
 	// AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization
 	// request flow with an OIDC identity provider. By default only the "openid" scope will be requested.
+	// +optional
 	AdditionalScopes []string `json:"additionalScopes"`
 }
 
@@ -47,10 +48,12 @@ type OIDCAuthorizationConfig struct {
 type OIDCClaims struct {
 	// Groups provides the name of the token claim that will be used to ascertain the groups to which
 	// an identity belongs.
+	// +optional
 	Groups string `json:"groups"`
 
 	// Username provides the name of the token claim that will be used to ascertain an identity's
 	// username.
+	// +optional
 	Username string `json:"username"`
 }
 
@@ -74,10 +77,12 @@ type UpstreamOIDCProviderSpec struct {
 
 	// AuthorizationConfig holds information about how to form the OAuth2 authorization request
 	// parameters to be used with this OIDC identity provider.
+	// +optional
 	AuthorizationConfig OIDCAuthorizationConfig `json:"authorizationConfig"`
 
 	// Claims provides the names of token claims that will be used when inspecting an identity from
 	// this OIDC identity provider.
+	// +optional
 	Claims OIDCClaims `json:"claims"`
 
 	// OIDCClient contains OIDC client information to be used used with this OIDC identity

generated/1.18/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml

@@ -64,8 +64,6 @@ spec:
                     items:
                       type: string
                     type: array
-                required:
-                - additionalScopes
                 type: object
               claims:
                 description: Claims provides the names of token claims that will be
@@ -79,9 +77,6 @@ spec:
                     description: Username provides the name of the token claim that
                       will be used to ascertain an identity's username.
                     type: string
-                required:
-                - groups
-                - username
                 type: object
               client:
                 description: OIDCClient contains OIDC client information to be used
@@ -104,8 +99,6 @@ spec:
                 pattern: ^https://
                 type: string
             required:
-            - authorizationConfig
-            - claims
             - client
             - issuer
             type: object

generated/1.19/apis/supervisor/idp/v1alpha1/types_upstreamoidcprovider.go

@@ -16,7 +16,7 @@ const (
 	// PhaseReady is the phase for an UpstreamOIDCProvider resource in a healthy state.
 	PhaseReady UpstreamOIDCProviderPhase = "Ready"
 
-	// PhaseErorr is the phase for an UpstreamOIDCProvider in an unhealthy state.
+	// PhaseError is the phase for an UpstreamOIDCProvider in an unhealthy state.
 	PhaseError UpstreamOIDCProviderPhase = "Error"
 )
 
@@ -40,6 +40,7 @@ type UpstreamOIDCProviderStatus struct {
 type OIDCAuthorizationConfig struct {
 	// AdditionalScopes are the scopes in addition to "openid" that will be requested as part of the authorization
 	// request flow with an OIDC identity provider. By default only the "openid" scope will be requested.
+	// +optional
 	AdditionalScopes []string `json:"additionalScopes"`
 }
 
@@ -47,10 +48,12 @@ type OIDCAuthorizationConfig struct {
 type OIDCClaims struct {
 	// Groups provides the name of the token claim that will be used to ascertain the groups to which
 	// an identity belongs.
+	// +optional
 	Groups string `json:"groups"`
 
 	// Username provides the name of the token claim that will be used to ascertain an identity's
 	// username.
+	// +optional
 	Username string `json:"username"`
 }
 
@@ -74,10 +77,12 @@ type UpstreamOIDCProviderSpec struct {
 
 	// AuthorizationConfig holds information about how to form the OAuth2 authorization request
 	// parameters to be used with this OIDC identity provider.
+	// +optional
 	AuthorizationConfig OIDCAuthorizationConfig `json:"authorizationConfig"`
 
 	// Claims provides the names of token claims that will be used when inspecting an identity from
 	// this OIDC identity provider.
+	// +optional
 	Claims OIDCClaims `json:"claims"`
 
 	// OIDCClient contains OIDC client information to be used used with this OIDC identity

generated/1.19/crds/idp.supervisor.pinniped.dev_upstreamoidcproviders.yaml

@@ -64,8 +64,6 @@ spec:
                     items:
                       type: string
                     type: array
-                required:
-                - additionalScopes
                 type: object
               claims:
                 description: Claims provides the names of token claims that will be
@@ -79,9 +77,6 @@ spec:
                     description: Username provides the name of the token claim that
                       will be used to ascertain an identity's username.
                     type: string
-                required:
-                - groups
-                - username
                 type: object
               client:
                 description: OIDCClient contains OIDC client information to be used
@@ -104,8 +99,6 @@ spec:
                 pattern: ^https://
                 type: string
             required:
-            - authorizationConfig
-            - claims
             - client
             - issuer
             type: object

internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"net/url"
 	"sort"
-	"strings"
 	"time"
 
 	"github.com/coreos/go-oidc"
@@ -208,12 +207,11 @@ func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.Upst
 		var err error
 		discoveredProvider, err = oidc.NewProvider(ctx, upstream.Spec.Issuer)
 		if err != nil {
-			err := fmt.Errorf("failed to perform OIDC discovery against %q: %w", upstream.Spec.Issuer, err)
 			return &v1alpha1.Condition{
 				Type:    typeOIDCDiscoverySucceeded,
 				Status:  v1alpha1.ConditionFalse,
 				Reason:  reasonUnreachable,
-				Message: strings.TrimSpace(err.Error()),
+				Message: fmt.Sprintf("failed to perform OIDC discovery against %q", upstream.Spec.Issuer),
 			}
 		}
 

internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go

@@ -208,8 +208,8 @@ func TestController(t *testing.T) {
 			wantErr: controllerlib.ErrSyntheticRequeue.Error(),
 			wantLogs: []string{
 				`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`,
-				`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="failed to perform OIDC discovery against \"invalid-url\": Get \"invalid-url/.well-known/openid-configuration\": unsupported protocol scheme \"\"" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`,
+				`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="failed to perform OIDC discovery against \"invalid-url\"" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`,
-				`upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="failed to perform OIDC discovery against \"invalid-url\": Get \"invalid-url/.well-known/openid-configuration\": unsupported protocol scheme \"\"" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`,
+				`upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="failed to perform OIDC discovery against \"invalid-url\"" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`,
 			},
 			wantResultingCache: []provider.UpstreamOIDCIdentityProvider{},
 			wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{
@@ -229,7 +229,7 @@ func TestController(t *testing.T) {
 							Status:             "False",
 							LastTransitionTime: now,
 							Reason:             "Unreachable",
-							Message:            `failed to perform OIDC discovery against "invalid-url": Get "invalid-url/.well-known/openid-configuration": unsupported protocol scheme ""`,
+							Message:            `failed to perform OIDC discovery against "invalid-url"`,
 						},
 					},
 				},

test/integration/supervisor_upstream_test.go

@@ -42,7 +42,7 @@ func TestSupervisorUpstreamOIDCDiscovery(t *testing.T) {
 				Type:    "OIDCDiscoverySucceeded",
 				Status:  v1alpha1.ConditionFalse,
 				Reason:  "Unreachable",
-				Message: `failed to perform OIDC discovery against "https://127.0.0.1:444444/issuer": Get "https://127.0.0.1:444444/issuer/.well-known/openid-configuration": dial tcp: address 444444: invalid port`,
+				Message: `failed to perform OIDC discovery against "https://127.0.0.1:444444/issuer"`,
 			},
 		})
 	})