diff --git a/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go.tmpl b/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go.tmpl index 85498a70e..1a36a441b 100644 --- a/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go.tmpl +++ b/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go.tmpl @@ -52,8 +52,12 @@ type GitHubIdentityProviderStatus struct { // GitHubAPIConfig allows configuration for GitHub Enterprise Server type GitHubAPIConfig struct { // Host is required only for GitHub Enterprise Server. - // Defaults to using GitHub's public API (github.com). - // Do not specify a protocol or scheme since 'https://' will always be used. + // Defaults to using GitHub's public API ("github.com"). + // Do not specify a protocol or scheme since "https://" will always be used. + // Port is optional. Do not specify a path, query, fragment, or userinfo. + // Only domain name or IP address, subdomains (optional), and port (optional). + // IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + // in square brackets. Example: "[::1]:443". // // +kubebuilder:default="github.com" // +kubebuilder:validation:MinLength=1 diff --git a/deploy/supervisor/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/deploy/supervisor/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 12b9a807f..40a300718 100644 --- a/deploy/supervisor/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/deploy/supervisor/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -208,8 +208,12 @@ spec: default: github.com description: |- Host is required only for GitHub Enterprise Server. - Defaults to using GitHub's public API (github.com). - Do not specify a protocol or scheme since 'https://' will always be used. + Defaults to using GitHub's public API ("github.com"). + Do not specify a protocol or scheme since "https://" will always be used. + Port is optional. Do not specify a path, query, fragment, or userinfo. + Only domain name or IP address, subdomains (optional), and port (optional). + IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + in square brackets. Example: "[::1]:443". minLength: 1 type: string tls: diff --git a/generated/1.21/README.adoc b/generated/1.21/README.adoc index 0934d8d11..547afeae9 100644 --- a/generated/1.21/README.adoc +++ b/generated/1.21/README.adoc @@ -1392,7 +1392,7 @@ GitHubAPIConfig allows configuration for GitHub Enterprise Server [cols="25a,75a", options="header"] |=== | Field | Description -| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API (github.com). Do not specify a protocol or scheme since 'https://' will always be used. +| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API ("github.com"). Do not specify a protocol or scheme since "https://" will always be used. Port is optional. Do not specify a path, query, fragment, or userinfo. Only domain name or IP address, subdomains (optional), and port (optional). IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address in square brackets. Example: "[::1]:443". | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-21-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for GitHub Enterprise Server. |=== diff --git a/generated/1.21/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.21/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 85498a70e..1a36a441b 100644 --- a/generated/1.21/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.21/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -52,8 +52,12 @@ type GitHubIdentityProviderStatus struct { // GitHubAPIConfig allows configuration for GitHub Enterprise Server type GitHubAPIConfig struct { // Host is required only for GitHub Enterprise Server. - // Defaults to using GitHub's public API (github.com). - // Do not specify a protocol or scheme since 'https://' will always be used. + // Defaults to using GitHub's public API ("github.com"). + // Do not specify a protocol or scheme since "https://" will always be used. + // Port is optional. Do not specify a path, query, fragment, or userinfo. + // Only domain name or IP address, subdomains (optional), and port (optional). + // IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + // in square brackets. Example: "[::1]:443". // // +kubebuilder:default="github.com" // +kubebuilder:validation:MinLength=1 diff --git a/generated/1.21/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.21/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 44b5d6609..21ab3e8ff 100644 --- a/generated/1.21/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.21/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -208,8 +208,12 @@ spec: default: github.com description: |- Host is required only for GitHub Enterprise Server. - Defaults to using GitHub's public API (github.com). - Do not specify a protocol or scheme since 'https://' will always be used. + Defaults to using GitHub's public API ("github.com"). + Do not specify a protocol or scheme since "https://" will always be used. + Port is optional. Do not specify a path, query, fragment, or userinfo. + Only domain name or IP address, subdomains (optional), and port (optional). + IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + in square brackets. Example: "[::1]:443". minLength: 1 type: string tls: diff --git a/generated/1.22/README.adoc b/generated/1.22/README.adoc index a90ba876c..4ff874eff 100644 --- a/generated/1.22/README.adoc +++ b/generated/1.22/README.adoc @@ -1392,7 +1392,7 @@ GitHubAPIConfig allows configuration for GitHub Enterprise Server [cols="25a,75a", options="header"] |=== | Field | Description -| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API (github.com). Do not specify a protocol or scheme since 'https://' will always be used. +| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API ("github.com"). Do not specify a protocol or scheme since "https://" will always be used. Port is optional. Do not specify a path, query, fragment, or userinfo. Only domain name or IP address, subdomains (optional), and port (optional). IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address in square brackets. Example: "[::1]:443". | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-22-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for GitHub Enterprise Server. |=== diff --git a/generated/1.22/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.22/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 85498a70e..1a36a441b 100644 --- a/generated/1.22/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.22/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -52,8 +52,12 @@ type GitHubIdentityProviderStatus struct { // GitHubAPIConfig allows configuration for GitHub Enterprise Server type GitHubAPIConfig struct { // Host is required only for GitHub Enterprise Server. - // Defaults to using GitHub's public API (github.com). - // Do not specify a protocol or scheme since 'https://' will always be used. + // Defaults to using GitHub's public API ("github.com"). + // Do not specify a protocol or scheme since "https://" will always be used. + // Port is optional. Do not specify a path, query, fragment, or userinfo. + // Only domain name or IP address, subdomains (optional), and port (optional). + // IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + // in square brackets. Example: "[::1]:443". // // +kubebuilder:default="github.com" // +kubebuilder:validation:MinLength=1 diff --git a/generated/1.22/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.22/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 44b5d6609..21ab3e8ff 100644 --- a/generated/1.22/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.22/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -208,8 +208,12 @@ spec: default: github.com description: |- Host is required only for GitHub Enterprise Server. - Defaults to using GitHub's public API (github.com). - Do not specify a protocol or scheme since 'https://' will always be used. + Defaults to using GitHub's public API ("github.com"). + Do not specify a protocol or scheme since "https://" will always be used. + Port is optional. Do not specify a path, query, fragment, or userinfo. + Only domain name or IP address, subdomains (optional), and port (optional). + IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + in square brackets. Example: "[::1]:443". minLength: 1 type: string tls: diff --git a/generated/1.23/README.adoc b/generated/1.23/README.adoc index f9d05ea77..c6c2d3a9e 100644 --- a/generated/1.23/README.adoc +++ b/generated/1.23/README.adoc @@ -1392,7 +1392,7 @@ GitHubAPIConfig allows configuration for GitHub Enterprise Server [cols="25a,75a", options="header"] |=== | Field | Description -| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API (github.com). Do not specify a protocol or scheme since 'https://' will always be used. +| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API ("github.com"). Do not specify a protocol or scheme since "https://" will always be used. Port is optional. Do not specify a path, query, fragment, or userinfo. Only domain name or IP address, subdomains (optional), and port (optional). IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address in square brackets. Example: "[::1]:443". | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-23-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for GitHub Enterprise Server. |=== diff --git a/generated/1.23/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.23/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 85498a70e..1a36a441b 100644 --- a/generated/1.23/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.23/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -52,8 +52,12 @@ type GitHubIdentityProviderStatus struct { // GitHubAPIConfig allows configuration for GitHub Enterprise Server type GitHubAPIConfig struct { // Host is required only for GitHub Enterprise Server. - // Defaults to using GitHub's public API (github.com). - // Do not specify a protocol or scheme since 'https://' will always be used. + // Defaults to using GitHub's public API ("github.com"). + // Do not specify a protocol or scheme since "https://" will always be used. + // Port is optional. Do not specify a path, query, fragment, or userinfo. + // Only domain name or IP address, subdomains (optional), and port (optional). + // IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + // in square brackets. Example: "[::1]:443". // // +kubebuilder:default="github.com" // +kubebuilder:validation:MinLength=1 diff --git a/generated/1.23/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.23/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 12b9a807f..40a300718 100644 --- a/generated/1.23/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.23/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -208,8 +208,12 @@ spec: default: github.com description: |- Host is required only for GitHub Enterprise Server. - Defaults to using GitHub's public API (github.com). - Do not specify a protocol or scheme since 'https://' will always be used. + Defaults to using GitHub's public API ("github.com"). + Do not specify a protocol or scheme since "https://" will always be used. + Port is optional. Do not specify a path, query, fragment, or userinfo. + Only domain name or IP address, subdomains (optional), and port (optional). + IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + in square brackets. Example: "[::1]:443". minLength: 1 type: string tls: diff --git a/generated/1.24/README.adoc b/generated/1.24/README.adoc index a785fa51d..864aee79e 100644 --- a/generated/1.24/README.adoc +++ b/generated/1.24/README.adoc @@ -1392,7 +1392,7 @@ GitHubAPIConfig allows configuration for GitHub Enterprise Server [cols="25a,75a", options="header"] |=== | Field | Description -| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API (github.com). Do not specify a protocol or scheme since 'https://' will always be used. +| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API ("github.com"). Do not specify a protocol or scheme since "https://" will always be used. Port is optional. Do not specify a path, query, fragment, or userinfo. Only domain name or IP address, subdomains (optional), and port (optional). IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address in square brackets. Example: "[::1]:443". | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-24-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for GitHub Enterprise Server. |=== diff --git a/generated/1.24/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.24/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 85498a70e..1a36a441b 100644 --- a/generated/1.24/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.24/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -52,8 +52,12 @@ type GitHubIdentityProviderStatus struct { // GitHubAPIConfig allows configuration for GitHub Enterprise Server type GitHubAPIConfig struct { // Host is required only for GitHub Enterprise Server. - // Defaults to using GitHub's public API (github.com). - // Do not specify a protocol or scheme since 'https://' will always be used. + // Defaults to using GitHub's public API ("github.com"). + // Do not specify a protocol or scheme since "https://" will always be used. + // Port is optional. Do not specify a path, query, fragment, or userinfo. + // Only domain name or IP address, subdomains (optional), and port (optional). + // IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + // in square brackets. Example: "[::1]:443". // // +kubebuilder:default="github.com" // +kubebuilder:validation:MinLength=1 diff --git a/generated/1.24/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.24/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 12b9a807f..40a300718 100644 --- a/generated/1.24/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.24/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -208,8 +208,12 @@ spec: default: github.com description: |- Host is required only for GitHub Enterprise Server. - Defaults to using GitHub's public API (github.com). - Do not specify a protocol or scheme since 'https://' will always be used. + Defaults to using GitHub's public API ("github.com"). + Do not specify a protocol or scheme since "https://" will always be used. + Port is optional. Do not specify a path, query, fragment, or userinfo. + Only domain name or IP address, subdomains (optional), and port (optional). + IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + in square brackets. Example: "[::1]:443". minLength: 1 type: string tls: diff --git a/generated/1.25/README.adoc b/generated/1.25/README.adoc index 34ea4dec9..f2b4d0e5e 100644 --- a/generated/1.25/README.adoc +++ b/generated/1.25/README.adoc @@ -1392,7 +1392,7 @@ GitHubAPIConfig allows configuration for GitHub Enterprise Server [cols="25a,75a", options="header"] |=== | Field | Description -| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API (github.com). Do not specify a protocol or scheme since 'https://' will always be used. +| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API ("github.com"). Do not specify a protocol or scheme since "https://" will always be used. Port is optional. Do not specify a path, query, fragment, or userinfo. Only domain name or IP address, subdomains (optional), and port (optional). IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address in square brackets. Example: "[::1]:443". | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-25-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for GitHub Enterprise Server. |=== diff --git a/generated/1.25/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.25/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 85498a70e..1a36a441b 100644 --- a/generated/1.25/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.25/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -52,8 +52,12 @@ type GitHubIdentityProviderStatus struct { // GitHubAPIConfig allows configuration for GitHub Enterprise Server type GitHubAPIConfig struct { // Host is required only for GitHub Enterprise Server. - // Defaults to using GitHub's public API (github.com). - // Do not specify a protocol or scheme since 'https://' will always be used. + // Defaults to using GitHub's public API ("github.com"). + // Do not specify a protocol or scheme since "https://" will always be used. + // Port is optional. Do not specify a path, query, fragment, or userinfo. + // Only domain name or IP address, subdomains (optional), and port (optional). + // IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + // in square brackets. Example: "[::1]:443". // // +kubebuilder:default="github.com" // +kubebuilder:validation:MinLength=1 diff --git a/generated/1.25/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.25/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 12b9a807f..40a300718 100644 --- a/generated/1.25/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.25/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -208,8 +208,12 @@ spec: default: github.com description: |- Host is required only for GitHub Enterprise Server. - Defaults to using GitHub's public API (github.com). - Do not specify a protocol or scheme since 'https://' will always be used. + Defaults to using GitHub's public API ("github.com"). + Do not specify a protocol or scheme since "https://" will always be used. + Port is optional. Do not specify a path, query, fragment, or userinfo. + Only domain name or IP address, subdomains (optional), and port (optional). + IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + in square brackets. Example: "[::1]:443". minLength: 1 type: string tls: diff --git a/generated/1.26/README.adoc b/generated/1.26/README.adoc index 575243c65..b28b110f4 100644 --- a/generated/1.26/README.adoc +++ b/generated/1.26/README.adoc @@ -1392,7 +1392,7 @@ GitHubAPIConfig allows configuration for GitHub Enterprise Server [cols="25a,75a", options="header"] |=== | Field | Description -| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API (github.com). Do not specify a protocol or scheme since 'https://' will always be used. +| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API ("github.com"). Do not specify a protocol or scheme since "https://" will always be used. Port is optional. Do not specify a path, query, fragment, or userinfo. Only domain name or IP address, subdomains (optional), and port (optional). IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address in square brackets. Example: "[::1]:443". | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-26-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for GitHub Enterprise Server. |=== diff --git a/generated/1.26/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.26/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 85498a70e..1a36a441b 100644 --- a/generated/1.26/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.26/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -52,8 +52,12 @@ type GitHubIdentityProviderStatus struct { // GitHubAPIConfig allows configuration for GitHub Enterprise Server type GitHubAPIConfig struct { // Host is required only for GitHub Enterprise Server. - // Defaults to using GitHub's public API (github.com). - // Do not specify a protocol or scheme since 'https://' will always be used. + // Defaults to using GitHub's public API ("github.com"). + // Do not specify a protocol or scheme since "https://" will always be used. + // Port is optional. Do not specify a path, query, fragment, or userinfo. + // Only domain name or IP address, subdomains (optional), and port (optional). + // IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + // in square brackets. Example: "[::1]:443". // // +kubebuilder:default="github.com" // +kubebuilder:validation:MinLength=1 diff --git a/generated/1.26/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.26/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 12b9a807f..40a300718 100644 --- a/generated/1.26/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.26/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -208,8 +208,12 @@ spec: default: github.com description: |- Host is required only for GitHub Enterprise Server. - Defaults to using GitHub's public API (github.com). - Do not specify a protocol or scheme since 'https://' will always be used. + Defaults to using GitHub's public API ("github.com"). + Do not specify a protocol or scheme since "https://" will always be used. + Port is optional. Do not specify a path, query, fragment, or userinfo. + Only domain name or IP address, subdomains (optional), and port (optional). + IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + in square brackets. Example: "[::1]:443". minLength: 1 type: string tls: diff --git a/generated/1.27/README.adoc b/generated/1.27/README.adoc index 7454774e3..9336b4408 100644 --- a/generated/1.27/README.adoc +++ b/generated/1.27/README.adoc @@ -1392,7 +1392,7 @@ GitHubAPIConfig allows configuration for GitHub Enterprise Server [cols="25a,75a", options="header"] |=== | Field | Description -| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API (github.com). Do not specify a protocol or scheme since 'https://' will always be used. +| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API ("github.com"). Do not specify a protocol or scheme since "https://" will always be used. Port is optional. Do not specify a path, query, fragment, or userinfo. Only domain name or IP address, subdomains (optional), and port (optional). IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address in square brackets. Example: "[::1]:443". | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-27-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for GitHub Enterprise Server. |=== diff --git a/generated/1.27/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.27/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 85498a70e..1a36a441b 100644 --- a/generated/1.27/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.27/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -52,8 +52,12 @@ type GitHubIdentityProviderStatus struct { // GitHubAPIConfig allows configuration for GitHub Enterprise Server type GitHubAPIConfig struct { // Host is required only for GitHub Enterprise Server. - // Defaults to using GitHub's public API (github.com). - // Do not specify a protocol or scheme since 'https://' will always be used. + // Defaults to using GitHub's public API ("github.com"). + // Do not specify a protocol or scheme since "https://" will always be used. + // Port is optional. Do not specify a path, query, fragment, or userinfo. + // Only domain name or IP address, subdomains (optional), and port (optional). + // IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + // in square brackets. Example: "[::1]:443". // // +kubebuilder:default="github.com" // +kubebuilder:validation:MinLength=1 diff --git a/generated/1.27/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.27/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 12b9a807f..40a300718 100644 --- a/generated/1.27/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.27/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -208,8 +208,12 @@ spec: default: github.com description: |- Host is required only for GitHub Enterprise Server. - Defaults to using GitHub's public API (github.com). - Do not specify a protocol or scheme since 'https://' will always be used. + Defaults to using GitHub's public API ("github.com"). + Do not specify a protocol or scheme since "https://" will always be used. + Port is optional. Do not specify a path, query, fragment, or userinfo. + Only domain name or IP address, subdomains (optional), and port (optional). + IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + in square brackets. Example: "[::1]:443". minLength: 1 type: string tls: diff --git a/generated/1.28/README.adoc b/generated/1.28/README.adoc index c51bb6de6..a011d2218 100644 --- a/generated/1.28/README.adoc +++ b/generated/1.28/README.adoc @@ -1392,7 +1392,7 @@ GitHubAPIConfig allows configuration for GitHub Enterprise Server [cols="25a,75a", options="header"] |=== | Field | Description -| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API (github.com). Do not specify a protocol or scheme since 'https://' will always be used. +| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API ("github.com"). Do not specify a protocol or scheme since "https://" will always be used. Port is optional. Do not specify a path, query, fragment, or userinfo. Only domain name or IP address, subdomains (optional), and port (optional). IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address in square brackets. Example: "[::1]:443". | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-28-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for GitHub Enterprise Server. |=== diff --git a/generated/1.28/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.28/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 85498a70e..1a36a441b 100644 --- a/generated/1.28/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.28/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -52,8 +52,12 @@ type GitHubIdentityProviderStatus struct { // GitHubAPIConfig allows configuration for GitHub Enterprise Server type GitHubAPIConfig struct { // Host is required only for GitHub Enterprise Server. - // Defaults to using GitHub's public API (github.com). - // Do not specify a protocol or scheme since 'https://' will always be used. + // Defaults to using GitHub's public API ("github.com"). + // Do not specify a protocol or scheme since "https://" will always be used. + // Port is optional. Do not specify a path, query, fragment, or userinfo. + // Only domain name or IP address, subdomains (optional), and port (optional). + // IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + // in square brackets. Example: "[::1]:443". // // +kubebuilder:default="github.com" // +kubebuilder:validation:MinLength=1 diff --git a/generated/1.28/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.28/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 12b9a807f..40a300718 100644 --- a/generated/1.28/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.28/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -208,8 +208,12 @@ spec: default: github.com description: |- Host is required only for GitHub Enterprise Server. - Defaults to using GitHub's public API (github.com). - Do not specify a protocol or scheme since 'https://' will always be used. + Defaults to using GitHub's public API ("github.com"). + Do not specify a protocol or scheme since "https://" will always be used. + Port is optional. Do not specify a path, query, fragment, or userinfo. + Only domain name or IP address, subdomains (optional), and port (optional). + IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + in square brackets. Example: "[::1]:443". minLength: 1 type: string tls: diff --git a/generated/1.29/README.adoc b/generated/1.29/README.adoc index ea7d33f67..43aede20a 100644 --- a/generated/1.29/README.adoc +++ b/generated/1.29/README.adoc @@ -1392,7 +1392,7 @@ GitHubAPIConfig allows configuration for GitHub Enterprise Server [cols="25a,75a", options="header"] |=== | Field | Description -| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API (github.com). Do not specify a protocol or scheme since 'https://' will always be used. +| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API ("github.com"). Do not specify a protocol or scheme since "https://" will always be used. Port is optional. Do not specify a path, query, fragment, or userinfo. Only domain name or IP address, subdomains (optional), and port (optional). IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address in square brackets. Example: "[::1]:443". | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-29-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for GitHub Enterprise Server. |=== diff --git a/generated/1.29/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/1.29/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 85498a70e..1a36a441b 100644 --- a/generated/1.29/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/1.29/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -52,8 +52,12 @@ type GitHubIdentityProviderStatus struct { // GitHubAPIConfig allows configuration for GitHub Enterprise Server type GitHubAPIConfig struct { // Host is required only for GitHub Enterprise Server. - // Defaults to using GitHub's public API (github.com). - // Do not specify a protocol or scheme since 'https://' will always be used. + // Defaults to using GitHub's public API ("github.com"). + // Do not specify a protocol or scheme since "https://" will always be used. + // Port is optional. Do not specify a path, query, fragment, or userinfo. + // Only domain name or IP address, subdomains (optional), and port (optional). + // IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + // in square brackets. Example: "[::1]:443". // // +kubebuilder:default="github.com" // +kubebuilder:validation:MinLength=1 diff --git a/generated/1.29/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml b/generated/1.29/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml index 12b9a807f..40a300718 100644 --- a/generated/1.29/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml +++ b/generated/1.29/crds/idp.supervisor.pinniped.dev_githubidentityproviders.yaml @@ -208,8 +208,12 @@ spec: default: github.com description: |- Host is required only for GitHub Enterprise Server. - Defaults to using GitHub's public API (github.com). - Do not specify a protocol or scheme since 'https://' will always be used. + Defaults to using GitHub's public API ("github.com"). + Do not specify a protocol or scheme since "https://" will always be used. + Port is optional. Do not specify a path, query, fragment, or userinfo. + Only domain name or IP address, subdomains (optional), and port (optional). + IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + in square brackets. Example: "[::1]:443". minLength: 1 type: string tls: diff --git a/generated/latest/README.adoc b/generated/latest/README.adoc index ea7d33f67..43aede20a 100644 --- a/generated/latest/README.adoc +++ b/generated/latest/README.adoc @@ -1392,7 +1392,7 @@ GitHubAPIConfig allows configuration for GitHub Enterprise Server [cols="25a,75a", options="header"] |=== | Field | Description -| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API (github.com). Do not specify a protocol or scheme since 'https://' will always be used. +| *`host`* __string__ | Host is required only for GitHub Enterprise Server. Defaults to using GitHub's public API ("github.com"). Do not specify a protocol or scheme since "https://" will always be used. Port is optional. Do not specify a path, query, fragment, or userinfo. Only domain name or IP address, subdomains (optional), and port (optional). IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address in square brackets. Example: "[::1]:443". | *`tls`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-29-apis-supervisor-idp-v1alpha1-tlsspec[$$TLSSpec$$]__ | TLS configuration for GitHub Enterprise Server. |=== diff --git a/generated/latest/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go b/generated/latest/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go index 85498a70e..1a36a441b 100644 --- a/generated/latest/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go +++ b/generated/latest/apis/supervisor/idp/v1alpha1/types_githubidentityprovider.go @@ -52,8 +52,12 @@ type GitHubIdentityProviderStatus struct { // GitHubAPIConfig allows configuration for GitHub Enterprise Server type GitHubAPIConfig struct { // Host is required only for GitHub Enterprise Server. - // Defaults to using GitHub's public API (github.com). - // Do not specify a protocol or scheme since 'https://' will always be used. + // Defaults to using GitHub's public API ("github.com"). + // Do not specify a protocol or scheme since "https://" will always be used. + // Port is optional. Do not specify a path, query, fragment, or userinfo. + // Only domain name or IP address, subdomains (optional), and port (optional). + // IPv4 and IPv6 are supported. If using an IPv6 address with a port, you must enclose the IPv6 address + // in square brackets. Example: "[::1]:443". // // +kubebuilder:default="github.com" // +kubebuilder:validation:MinLength=1 diff --git a/internal/controller/authenticator/authenticator.go b/internal/controller/authenticator/authenticator.go index 7623aecd9..cab5e97b4 100644 --- a/internal/controller/authenticator/authenticator.go +++ b/internal/controller/authenticator/authenticator.go @@ -1,19 +1,9 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package authenticator contains helper code for dealing with *Authenticator CRDs. package authenticator -import ( - "crypto/x509" - "encoding/base64" - "fmt" - - "k8s.io/client-go/util/cert" - - auth1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1" -) - // Closer is a type that can be closed idempotently. // // This type is slightly different from io.Closer, because io.Closer can return an error and is not @@ -21,24 +11,3 @@ import ( type Closer interface { Close() } - -// CABundle returns a PEM-encoded CA bundle from the provided spec. If the provided spec is nil, a -// nil CA bundle will be returned. If the provided spec contains a CA bundle that is not properly -// encoded, an error will be returned. -func CABundle(spec *auth1alpha1.TLSSpec) (*x509.CertPool, []byte, error) { - if spec == nil || len(spec.CertificateAuthorityData) == 0 { - return nil, nil, nil - } - - pem, err := base64.StdEncoding.DecodeString(spec.CertificateAuthorityData) - if err != nil { - return nil, nil, err - } - - rootCAs, err := cert.NewPoolFromBytes(pem) - if err != nil { - return nil, nil, fmt.Errorf("certificateAuthorityData is not valid PEM: %w", err) - } - - return rootCAs, pem, nil -} diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go index 951db2bfa..9aa590647 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller.go @@ -240,7 +240,7 @@ func (c *jwtCacheFillerController) extractValueAsJWTAuthenticator(value authncac } func (c *jwtCacheFillerController) validateTLS(tlsSpec *auth1alpha1.TLSSpec, conditions []*metav1.Condition) (*x509.CertPool, []*metav1.Condition, bool) { - rootCAs, _, err := pinnipedauthenticator.CABundle(tlsSpec) + rootCAs, _, err := pinnipedcontroller.BuildCertPoolAuth(tlsSpec) if err != nil { msg := fmt.Sprintf("%s: %s", "invalid TLS configuration", err.Error()) conditions = append(conditions, &metav1.Condition{ @@ -594,7 +594,7 @@ func (c *jwtCacheFillerController) updateStatus( }) } - _ = conditionsutil.MergeConfigConditions( + _ = conditionsutil.MergeConditions( conditions, original.Generation, &updated.Status.Conditions, diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go index a6ebd96a9..3ac96c3eb 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go @@ -30,7 +30,6 @@ import ( conciergeclientset "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned" authinformers "go.pinniped.dev/generated/latest/client/concierge/informers/externalversions/authentication/v1alpha1" pinnipedcontroller "go.pinniped.dev/internal/controller" - pinnipedauthenticator "go.pinniped.dev/internal/controller/authenticator" "go.pinniped.dev/internal/controller/authenticator/authncache" "go.pinniped.dev/internal/controller/conditionsutil" "go.pinniped.dev/internal/controllerlib" @@ -316,7 +315,7 @@ func (c *webhookCacheFillerController) validateConnection(certPool *x509.CertPoo } func (c *webhookCacheFillerController) validateTLSBundle(tlsSpec *auth1alpha1.TLSSpec, conditions []*metav1.Condition) (*x509.CertPool, []byte, []*metav1.Condition, bool) { - rootCAs, pemBytes, err := pinnipedauthenticator.CABundle(tlsSpec) + rootCAs, pemBytes, err := pinnipedcontroller.BuildCertPoolAuth(tlsSpec) if err != nil { msg := fmt.Sprintf("%s: %s", "invalid TLS configuration", err.Error()) conditions = append(conditions, &metav1.Condition{ @@ -411,7 +410,7 @@ func (c *webhookCacheFillerController) updateStatus( }) } - _ = conditionsutil.MergeConfigConditions( + _ = conditionsutil.MergeConditions( conditions, original.Generation, &updated.Status.Conditions, diff --git a/internal/controller/conditionsutil/conditions_util.go b/internal/controller/conditionsutil/conditions_util.go index 7412db764..38004b907 100644 --- a/internal/controller/conditionsutil/conditions_util.go +++ b/internal/controller/conditionsutil/conditions_util.go @@ -12,29 +12,34 @@ import ( "go.pinniped.dev/internal/plog" ) -// MergeIDPConditions merges conditions into conditionsToUpdate. If returns true if it merged any error conditions. -func MergeIDPConditions(conditions []*metav1.Condition, observedGeneration int64, conditionsToUpdate *[]metav1.Condition, log plog.MinLogger) bool { - hadErrorCondition := false +// MergeConditions merges conditions into conditionsToUpdate. +// Note that LastTransitionTime refers to the time when the status changed, +// but ObservedGeneration should be the current generation for all conditions, since Pinniped should always check every condition. +// It returns true if any resulting condition has non-true status. +func MergeConditions( + conditions []*metav1.Condition, + observedGeneration int64, + conditionsToUpdate *[]metav1.Condition, + log plog.MinLogger, + lastTransitionTime metav1.Time, +) bool { for i := range conditions { cond := conditions[i].DeepCopy() - cond.LastTransitionTime = metav1.Now() + cond.LastTransitionTime = lastTransitionTime cond.ObservedGeneration = observedGeneration - if mergeIDPCondition(conditionsToUpdate, cond) { + if mergeCondition(conditionsToUpdate, cond) { log.Info("updated condition", "type", cond.Type, "status", cond.Status, "reason", cond.Reason, "message", cond.Message) } - if cond.Status == metav1.ConditionFalse { - hadErrorCondition = true - } } sort.SliceStable(*conditionsToUpdate, func(i, j int) bool { return (*conditionsToUpdate)[i].Type < (*conditionsToUpdate)[j].Type }) - return hadErrorCondition + return HadErrorCondition(conditions) } -// mergeIDPCondition merges a new metav1.Condition into a slice of existing conditions. It returns true +// mergeCondition merges a new metav1.Condition into a slice of existing conditions. It returns true // if the condition has meaningfully changed. -func mergeIDPCondition(existing *[]metav1.Condition, new *metav1.Condition) bool { +func mergeCondition(existing *[]metav1.Condition, new *metav1.Condition) bool { // Find any existing condition with a matching type. var old *metav1.Condition for i := range *existing { @@ -62,61 +67,7 @@ func mergeIDPCondition(existing *[]metav1.Condition, new *metav1.Condition) bool return true } - // Otherwise the entry is already up to date. - return false -} - -// MergeConfigConditions merges conditions into conditionsToUpdate. It returns true if it merged any error conditions. -func MergeConfigConditions(conditions []*metav1.Condition, observedGeneration int64, conditionsToUpdate *[]metav1.Condition, log plog.MinLogger, now metav1.Time) bool { - hadErrorCondition := false - for i := range conditions { - cond := conditions[i].DeepCopy() - cond.LastTransitionTime = now - cond.ObservedGeneration = observedGeneration - if mergeConfigCondition(conditionsToUpdate, cond) { - log.Info("updated condition", "type", cond.Type, "status", cond.Status, "reason", cond.Reason, "message", cond.Message) - } - if cond.Status == metav1.ConditionFalse { - hadErrorCondition = true - } - } - sort.SliceStable(*conditionsToUpdate, func(i, j int) bool { - return (*conditionsToUpdate)[i].Type < (*conditionsToUpdate)[j].Type - }) - return hadErrorCondition -} - -// mergeConfigCondition merges a new metav1.Condition into a slice of existing conditions. It returns true -// if the condition has meaningfully changed. -func mergeConfigCondition(existing *[]metav1.Condition, new *metav1.Condition) bool { - // Find any existing condition with a matching type. - var old *metav1.Condition - for i := range *existing { - if (*existing)[i].Type == new.Type { - old = &(*existing)[i] - continue - } - } - - // If there is no existing condition of this type, append this one and we're done. - if old == nil { - *existing = append(*existing, *new) - return true - } - - // Set the LastTransitionTime depending on whether the status has changed. - new = new.DeepCopy() - if old.Status == new.Status { - new.LastTransitionTime = old.LastTransitionTime - } - - // If anything has actually changed, update the entry and return true. - if !equality.Semantic.DeepEqual(old, new) { - *old = *new - return true - } - - // Otherwise the entry is already up to date. + // Otherwise the entry is already up-to-date. return false } diff --git a/internal/controller/conditionsutil/conditions_util_test.go b/internal/controller/conditionsutil/conditions_util_test.go new file mode 100644 index 000000000..2049e9f2b --- /dev/null +++ b/internal/controller/conditionsutil/conditions_util_test.go @@ -0,0 +1,158 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package conditionsutil + +import ( + "bytes" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "go.pinniped.dev/internal/plog" +) + +func TestMergeIDPConditions(t *testing.T) { + twoHoursAgo := metav1.Time{Time: time.Now().Add(-2 * time.Hour)} + oneHourAgo := metav1.Time{Time: time.Now().Add(-1 * time.Hour)} + testTime := metav1.Now() + + tests := []struct { + name string + newConditions []*metav1.Condition + conditionsToUpdate *[]metav1.Condition + observedGeneration int64 + wantResult bool + wantLogSnippets []string + wantConditions []metav1.Condition + }{ + { + name: "True -> False returns true", + newConditions: []*metav1.Condition{ + { + Type: "UnchangedType", + Status: metav1.ConditionTrue, + Reason: "unchanged reason", + Message: "unchanged message", + }, + { + Type: "FalseToTrueType", + Status: metav1.ConditionFalse, + Reason: "new reason", + Message: "new message", + }, + { + Type: "NewType", + Status: metav1.ConditionTrue, + Reason: "new reason", + Message: "new message", + }, + }, + conditionsToUpdate: &[]metav1.Condition{ + { + Type: "UnchangedType", + Status: metav1.ConditionTrue, + ObservedGeneration: int64(10), + LastTransitionTime: twoHoursAgo, + Reason: "unchanged reason", + Message: "unchanged message", + }, + { + Type: "FalseToTrueType", + Status: metav1.ConditionTrue, + ObservedGeneration: int64(5), + LastTransitionTime: oneHourAgo, + Reason: "old reason", + Message: "old message", + }, + }, + observedGeneration: int64(100), + wantLogSnippets: []string{ + `"message":"updated condition","type":"UnchangedType","status":"True"`, + `"message":"updated condition","type":"NewType","status":"True"`, + `"message":"updated condition","type":"FalseToTrueType","status":"False"`, + }, + wantConditions: []metav1.Condition{ + { + Type: "FalseToTrueType", + Status: metav1.ConditionFalse, + ObservedGeneration: int64(100), + LastTransitionTime: testTime, + Reason: "new reason", + Message: "new message", + }, + { + Type: "NewType", + Status: metav1.ConditionTrue, + ObservedGeneration: int64(100), + LastTransitionTime: testTime, + Reason: "new reason", + Message: "new message", + }, + { + Type: "UnchangedType", + Status: metav1.ConditionTrue, + ObservedGeneration: int64(100), + LastTransitionTime: twoHoursAgo, + Reason: "unchanged reason", + Message: "unchanged message", + }, + }, + wantResult: true, + }, + { + name: "No logs when ObservedGeneration is unchanged", + newConditions: []*metav1.Condition{ + { + Type: "UnchangedType", + Status: metav1.ConditionFalse, + Reason: "unchanged reason", + Message: "unchanged message", + }, + }, + conditionsToUpdate: &[]metav1.Condition{ + { + Type: "UnchangedType", + Status: metav1.ConditionFalse, + ObservedGeneration: int64(10), + LastTransitionTime: twoHoursAgo, + Reason: "unchanged reason", + Message: "unchanged message", + }, + }, + observedGeneration: int64(10), + wantConditions: []metav1.Condition{ + { + Type: "UnchangedType", + Status: metav1.ConditionFalse, + ObservedGeneration: int64(10), + LastTransitionTime: twoHoursAgo, + Reason: "unchanged reason", + Message: "unchanged message", + }, + }, + wantResult: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + var log bytes.Buffer + logger := plog.TestLogger(t, &log) + + result := MergeConditions(tt.newConditions, tt.observedGeneration, tt.conditionsToUpdate, logger, testTime) + + logString := log.String() + require.Equal(t, len(tt.wantLogSnippets), strings.Count(logString, "\n")) + for _, wantLog := range tt.wantLogSnippets { + require.Contains(t, logString, wantLog) + } + require.Equal(t, tt.wantResult, result) + require.Equal(t, tt.wantConditions, *tt.conditionsToUpdate) + }) + } +} diff --git a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go index 2a127d450..42aa65097 100644 --- a/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go +++ b/internal/controller/supervisorconfig/activedirectoryupstreamwatcher/active_directory_upstream_watcher.go @@ -1,4 +1,4 @@ -// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package activedirectoryupstreamwatcher implements a controller which watches ActiveDirectoryIdentityProviders. @@ -368,7 +368,7 @@ func (c *activeDirectoryWatcherController) updateStatus(ctx context.Context, ups log := plog.WithValues("namespace", upstream.Namespace, "name", upstream.Name) updated := upstream.DeepCopy() - hadErrorCondition := conditionsutil.MergeIDPConditions(conditions, upstream.Generation, &updated.Status.Conditions, log) + hadErrorCondition := conditionsutil.MergeConditions(conditions, upstream.Generation, &updated.Status.Conditions, log, metav1.Now()) updated.Status.Phase = v1alpha1.ActiveDirectoryPhaseReady if hadErrorCondition { diff --git a/internal/controller/supervisorconfig/federation_domain_watcher.go b/internal/controller/supervisorconfig/federation_domain_watcher.go index 9152d0f6b..f571209ae 100644 --- a/internal/controller/supervisorconfig/federation_domain_watcher.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher.go @@ -813,7 +813,7 @@ func (c *federationDomainWatcherController) updateStatus( }) } - _ = conditionsutil.MergeConfigConditions(conditions, + _ = conditionsutil.MergeConditions(conditions, federationDomain.Generation, &updated.Status.Conditions, plog.New().WithName(controllerName), metav1.NewTime(c.clock.Now())) if equality.Semantic.DeepEqual(federationDomain, updated) { diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go index d1cd61bf1..3baa898f3 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go @@ -6,39 +6,54 @@ package githubupstreamwatcher import ( "context" + "crypto/tls" + "crypto/x509" + "errors" "fmt" + "net" + "net/http" + "slices" + "strings" + "golang.org/x/oauth2" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" + k8sapierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" errorsutil "k8s.io/apimachinery/pkg/util/errors" + k8sutilerrors "k8s.io/apimachinery/pkg/util/errors" corev1informers "k8s.io/client-go/informers/core/v1" + "k8s.io/utils/clock" "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" supervisorclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned" idpinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions/idp/v1alpha1" pinnipedcontroller "go.pinniped.dev/internal/controller" "go.pinniped.dev/internal/controller/conditionsutil" + "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/crypto/ptls" + "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/federationdomain/upstreamprovider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/upstreamgithub" ) const ( - // Setup for the name of our controller in logs. controllerName = "github-upstream-observer" // Constants related to the client credentials Secret. - gitHubClientSecretType corev1.SecretType = "secrets.pinniped.dev/github-client" + gitHubClientSecretType corev1.SecretType = "secrets.pinniped.dev/github-client" + clientIDDataKey, clientSecretDataKey string = "clientID", "clientSecret" - // - // clientIDDataKey = "clientID" - // clientSecretDataKey = "clientSecret" - // - // // Constants related to conditions. - // typeClientCredentialsValid = "ClientCredentialsValid" //nolint:gosec // this is not a credential. + HostValid string = "HostValid" + TLSConfigurationValid string = "TLSConfigurationValid" + OrganizationsPolicyValid string = "OrganizationsPolicyValid" + // ClientCredentialsObtained is different from other status conditions because it only checks that the client credentials + // have been obtained. The controller has no way to verify whether they are valid. + ClientCredentialsObtained string = "ClientCredentialsObtained" //nolint:gosec // this is not a credential + GitHubConnectionValid string = "GitHubConnectionValid" ) // UpstreamGitHubIdentityProviderICache is a thread safe cache that holds a list of validated upstream GitHub IDP configurations. @@ -47,40 +62,52 @@ type UpstreamGitHubIdentityProviderICache interface { } type gitHubWatcherController struct { + namespace string cache UpstreamGitHubIdentityProviderICache log plog.Logger client supervisorclientset.Interface gitHubIdentityProviderInformer idpinformers.GitHubIdentityProviderInformer secretInformer corev1informers.SecretInformer + httpClientBuilder func(rootCAs *x509.CertPool) *http.Client + clock clock.Clock } // New instantiates a new controllerlib.Controller which will populate the provided UpstreamGitHubIdentityProviderICache. func New( + namespace string, idpCache UpstreamGitHubIdentityProviderICache, client supervisorclientset.Interface, gitHubIdentityProviderInformer idpinformers.GitHubIdentityProviderInformer, secretInformer corev1informers.SecretInformer, log plog.Logger, withInformer pinnipedcontroller.WithInformerOptionFunc, + clock clock.Clock, + httpClientBuilder func(rootCAs *x509.CertPool) *http.Client, ) controllerlib.Controller { c := gitHubWatcherController{ + namespace: namespace, cache: idpCache, client: client, log: log.WithName(controllerName), gitHubIdentityProviderInformer: gitHubIdentityProviderInformer, secretInformer: secretInformer, + httpClientBuilder: httpClientBuilder, + clock: clock, } return controllerlib.New( controllerlib.Config{Name: controllerName, Syncer: &c}, withInformer( gitHubIdentityProviderInformer, - pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), + pinnipedcontroller.SimpleFilter(func(obj metav1.Object) bool { + gitHubIDP, ok := obj.(*v1alpha1.GitHubIdentityProvider) + return ok && gitHubIDP.Namespace == namespace + }, pinnipedcontroller.SingletonQueue()), controllerlib.InformerOption{}, ), withInformer( secretInformer, - pinnipedcontroller.MatchAnySecretOfTypeFilter(gitHubClientSecretType, pinnipedcontroller.SingletonQueue()), + pinnipedcontroller.MatchAnySecretOfTypeFilter(gitHubClientSecretType, pinnipedcontroller.SingletonQueue(), namespace), controllerlib.InformerOption{}, ), ) @@ -89,79 +116,358 @@ func New( // Sync implements controllerlib.Syncer. func (c *gitHubWatcherController) Sync(ctx controllerlib.Context) error { actualUpstreams, err := c.gitHubIdentityProviderInformer.Lister().List(labels.Everything()) - if err != nil { + if err != nil { // untested return fmt.Errorf("failed to list GitHubIdentityProviders: %w", err) } - var errs []error + // Sort them by name just so that the logs output is consistent + slices.SortStableFunc(actualUpstreams, func(a, b *v1alpha1.GitHubIdentityProvider) int { + return strings.Compare(a.Name, b.Name) + }) - requeue := false + var applicationErrors []error validatedUpstreams := make([]upstreamprovider.UpstreamGithubIdentityProviderI, 0, len(actualUpstreams)) for _, upstream := range actualUpstreams { - valid, err := c.validateUpstream(ctx, upstream) - if valid == nil { - requeue = true - errs = append(errs, err) - } else { - validatedUpstreams = append(validatedUpstreams, upstreamprovider.UpstreamGithubIdentityProviderI(valid)) + validatedUpstream, applicationErr := c.validateUpstreamAndUpdateConditions(ctx, upstream) + if applicationErr != nil { + applicationErrors = append(applicationErrors, applicationErr) + } else if validatedUpstream != nil { + validatedUpstreams = append(validatedUpstreams, validatedUpstream) } + // Else: + // If both validatedUpstream and applicationErr are nil, this must be because the upstream had configuration errors. + // This controller should take no action until the user has reconfigured the upstream. } c.cache.SetGitHubIdentityProviders(validatedUpstreams) - if requeue { - return controllerlib.ErrSyntheticRequeue + + // If we have recoverable application errors, let's do a requeue and capture all the applicationErrors too + if len(applicationErrors) > 0 { + applicationErrors = append([]error{controllerlib.ErrSyntheticRequeue}, applicationErrors...) } - // Sync loop errors: - // - Should not be configuration errors. Config errors a user must correct belong on the .Status - // object. The controller simply must wait for a user to correct before running again. - // - Other errors, such as networking errors, etc. are the types of errors that should return here - // and signal the controller to retry the sync loop. These may be corrected by machines. - return errorsutil.NewAggregate(errs) + return errorsutil.NewAggregate(applicationErrors) } -func (c *gitHubWatcherController) validateUpstream(ctx controllerlib.Context, upstream *v1alpha1.GitHubIdentityProvider) (*upstreamgithub.ProviderConfig, error) { - result := upstreamgithub.ProviderConfig{ - Name: upstream.Name, +func (c *gitHubWatcherController) validateClientSecret(secretName string) (*metav1.Condition, string, string, error) { + secret, unableToRetrieveSecretErr := c.secretInformer.Lister().Secrets(c.namespace).Get(secretName) + + // This error requires user interaction, so ignore it. + if k8sapierrors.IsNotFound(unableToRetrieveSecretErr) { + unableToRetrieveSecretErr = nil } - // TODO: once we firm up the proposal doc & merge, then firm up the CRD & merge, we can fill out these validations. + buildFalseCondition := func(prefix string) (*metav1.Condition, string, string, error) { + return &metav1.Condition{ + Type: ClientCredentialsObtained, + Status: metav1.ConditionFalse, + Reason: upstreamwatchers.ReasonNotFound, + Message: fmt.Sprintf("%s: secret from spec.client.SecretName (%q) must be found in namespace %q with type %q and keys %q and %q", + prefix, + secretName, + c.namespace, + gitHubClientSecretType, + clientIDDataKey, + clientSecretDataKey), + }, "", "", unableToRetrieveSecretErr + } + + if unableToRetrieveSecretErr != nil || secret == nil { + return buildFalseCondition(fmt.Sprintf("secret %q not found", secretName)) + } + + if secret.Type != gitHubClientSecretType { + return buildFalseCondition(fmt.Sprintf("wrong secret type %q", secret.Type)) + } + + clientID := string(secret.Data[clientIDDataKey]) + if len(clientID) < 1 { + return buildFalseCondition(fmt.Sprintf("missing key %q", clientIDDataKey)) + } + + clientSecret := string(secret.Data[clientSecretDataKey]) + if len(clientSecret) < 1 { + return buildFalseCondition(fmt.Sprintf("missing key %q", clientSecretDataKey)) + } + + if len(secret.Data) != 2 { + return buildFalseCondition("extra keys found") + } + + return &metav1.Condition{ + Type: ClientCredentialsObtained, + Status: metav1.ConditionTrue, + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf("clientID and clientSecret have been read from spec.client.SecretName (%q)", secretName), + }, clientID, clientSecret, nil +} + +func validateOrganizationsPolicy(organizationsSpec *v1alpha1.GitHubOrganizationsSpec) (*metav1.Condition, v1alpha1.GitHubAllowedAuthOrganizationsPolicy) { + var policy v1alpha1.GitHubAllowedAuthOrganizationsPolicy + if organizationsSpec.Policy != nil { + policy = *organizationsSpec.Policy + } + + // Should not happen due to CRD defaulting, enum validation, and CEL validation (for recent versions of K8s only!) + // That is why the message here is very minimal + if (policy == v1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers && len(organizationsSpec.Allowed) == 0) || + (policy == v1alpha1.GitHubAllowedAuthOrganizationsPolicyOnlyUsersFromAllowedOrganizations && len(organizationsSpec.Allowed) > 0) { + return &metav1.Condition{ + Type: OrganizationsPolicyValid, + Status: metav1.ConditionTrue, + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf("spec.allowAuthentication.organizations.policy (%q) is valid", policy), + }, policy + } + + if len(organizationsSpec.Allowed) > 0 { + return &metav1.Condition{ + Type: OrganizationsPolicyValid, + Status: metav1.ConditionFalse, + Reason: "Invalid", + Message: "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed", + }, policy + } + + return &metav1.Condition{ + Type: OrganizationsPolicyValid, + Status: metav1.ConditionFalse, + Reason: "Invalid", + Message: "spec.allowAuthentication.organizations.policy must be 'AllGitHubUsers' when spec.allowAuthentication.organizations.allowed is empty", + }, policy +} + +func (c *gitHubWatcherController) validateUpstreamAndUpdateConditions(ctx controllerlib.Context, upstream *v1alpha1.GitHubIdentityProvider) ( + *upstreamgithub.ProviderConfig, // If validated, returns the config + error, // This error will only refer to programmatic errors such as inability to perform a Dial or dereference a pointer, not configuration errors +) { + conditions := make([]*metav1.Condition, 0) + applicationErrors := make([]error, 0) + + clientSecretCondition, clientID, clientSecret, clientSecretErr := c.validateClientSecret(upstream.Spec.Client.SecretName) + conditions = append(conditions, clientSecretCondition) + if clientSecretErr != nil { // untested + applicationErrors = append(applicationErrors, clientSecretErr) + } + + // Should there be some sort of catch-all condition to capture this? + // This does not actually prevent a GitHub IDP from being added to the cache. + // CRD defaulting and validation should eliminate the possibility of an error here. + groupNameAttribute, usernameAttribute, userAndGroupErr := validateUserAndGroupAttributes(upstream) + if userAndGroupErr != nil { + applicationErrors = append(applicationErrors, userAndGroupErr) + } + + organizationPolicyCondition, policy := validateOrganizationsPolicy(&upstream.Spec.AllowAuthentication.Organizations) + conditions = append(conditions, organizationPolicyCondition) + + hostCondition, hostPort := validateHost(upstream.Spec.GitHubAPI) + conditions = append(conditions, hostCondition) + + tlsConfigCondition, certPool := c.validateTLSConfiguration(upstream.Spec.GitHubAPI.TLS) + conditions = append(conditions, tlsConfigCondition) + + githubConnectionCondition, hostURL, httpClient, githubConnectionErr := c.validateGitHubConnection( + hostPort, + certPool, + hostCondition.Status == metav1.ConditionTrue && tlsConfigCondition.Status == metav1.ConditionTrue, + ) + if githubConnectionErr != nil { + applicationErrors = append(applicationErrors, githubConnectionErr) + } + conditions = append(conditions, githubConnectionCondition) + // The critical pattern to maintain is that every run of the sync loop will populate the exact number of the exact - // same set of conditions. Conditions depending on other conditions should get Status: metav1.ConditionUnknown, or - // Status: metav1.ConditionFalse, never be omitted. - conditions := []*metav1.Condition{ - // we may opt to split this up into smaller validation functions. - // Each function should be responsible for validating one logical unit and setting one condition. - // c.validateGitHubAPI(), - // c.validateClaims(), - // c.validateAllowedAuthentication(), - // c.validateClient(), + // same set of conditions. Conditions depending on other conditions should get Status: metav1.ConditionUnknown, or + // Status: metav1.ConditionFalse, never be omitted. + if len(conditions) != 5 { // untested since all code paths result in 5 conditions + applicationErrors = append(applicationErrors, fmt.Errorf("expected %d conditions but found %d conditions", 5, len(conditions))) + return nil, k8sutilerrors.NewAggregate(applicationErrors) + } + hadErrorCondition, updateStatusErr := c.updateStatus(ctx.Context, upstream, conditions) + if updateStatusErr != nil { // untested + applicationErrors = append(applicationErrors, updateStatusErr) + } + // Any error condition means we will not add the IDP to the cache, so just return nil here + if hadErrorCondition { + return nil, k8sutilerrors.NewAggregate(applicationErrors) } - err := c.updateStatus(ctx.Context, upstream, conditions) - return &result, err + providerConfig := &upstreamgithub.ProviderConfig{ + Name: upstream.Name, + ResourceUID: upstream.UID, + Host: hostURL, + GroupNameAttribute: groupNameAttribute, + UsernameAttribute: usernameAttribute, + OAuth2Config: &oauth2.Config{ + ClientID: clientID, + ClientSecret: clientSecret, + }, + AllowedOrganizations: upstream.Spec.AllowAuthentication.Organizations.Allowed, + OrganizationLoginPolicy: policy, + AuthorizationURL: fmt.Sprintf("%s/login/oauth/authorize", hostURL), + HttpClient: httpClient, + } + return providerConfig, k8sutilerrors.NewAggregate(applicationErrors) +} + +func validateHost(gitHubAPIConfig v1alpha1.GitHubAPIConfig) (*metav1.Condition, *endpointaddr.HostPort) { + buildInvalidHost := func(host, reason string) *metav1.Condition { + return &metav1.Condition{ + Type: HostValid, + Status: metav1.ConditionFalse, + Reason: "InvalidHost", + Message: fmt.Sprintf("spec.githubAPI.host (%q) is not valid: %s", host, reason), + } + } + + // Should not happen due to CRD defaulting + if gitHubAPIConfig.Host == nil || len(*gitHubAPIConfig.Host) < 1 { + return buildInvalidHost("", "must not be empty"), nil + } + + host := *gitHubAPIConfig.Host + hostPort, addressParseErr := endpointaddr.Parse(host, 443) + if addressParseErr != nil { + // addressParseErr is not recoverable. It requires user interaction, so do not return the error. + return buildInvalidHost(host, addressParseErr.Error()), nil + } + + return &metav1.Condition{ + Type: HostValid, + Status: metav1.ConditionTrue, + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf("spec.githubAPI.host (%q) is valid", host), + }, &hostPort +} + +func (c *gitHubWatcherController) validateTLSConfiguration(tlsSpec *v1alpha1.TLSSpec) (*metav1.Condition, *x509.CertPool) { + certPool, _, buildCertPoolErr := pinnipedcontroller.BuildCertPoolIDP(tlsSpec) + if buildCertPoolErr != nil { + // buildCertPoolErr is not recoverable with a resync. + // It requires user interaction, so do not return the error. + return &metav1.Condition{ + Type: TLSConfigurationValid, + Status: metav1.ConditionFalse, + Reason: "InvalidTLSConfig", + Message: fmt.Sprintf("spec.githubAPI.tls.certificateAuthorityData is not valid: %s", buildCertPoolErr), + }, nil + } + + return &metav1.Condition{ + Type: TLSConfigurationValid, + Status: metav1.ConditionTrue, + Reason: upstreamwatchers.ReasonSuccess, + Message: "spec.githubAPI.tls.certificateAuthorityData is valid", + }, certPool +} + +func (c *gitHubWatcherController) validateGitHubConnection( + hostPort *endpointaddr.HostPort, + certPool *x509.CertPool, + validSoFar bool, +) (*metav1.Condition, string, *http.Client, error) { + if !validSoFar { + return &metav1.Condition{ + Type: GitHubConnectionValid, + Status: metav1.ConditionUnknown, + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + }, "", nil, nil + } + + conn, tlsDialErr := tls.Dial("tcp", hostPort.Endpoint(), ptls.Default(certPool)) + if tlsDialErr != nil { + return &metav1.Condition{ + Type: GitHubConnectionValid, + Status: metav1.ConditionFalse, + Reason: "UnableToDialServer", + Message: fmt.Sprintf("cannot dial server spec.githubAPI.host (%q): %s", hostPort.Endpoint(), buildDialErrorMessage(tlsDialErr)), + }, "", nil, tlsDialErr + } + + return &metav1.Condition{ + Type: GitHubConnectionValid, + Status: metav1.ConditionTrue, + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf("spec.githubAPI.host (%q) is reachable and TLS verification succeeds", hostPort.Endpoint()), + }, fmt.Sprintf("https://%s", hostPort.Endpoint()), c.httpClientBuilder(certPool), conn.Close() +} + +// buildDialErrorMessage standardizes DNS error messages that appear differently on different platforms, so that tests and log grepping is uniform. +func buildDialErrorMessage(tlsDialErr error) string { + reason := tlsDialErr.Error() + + var opError *net.OpError + var dnsError *net.DNSError + if errors.As(tlsDialErr, &opError) && errors.As(tlsDialErr, &dnsError) { + dnsError.Server = "" + opError.Err = dnsError + return opError.Error() + } + + return reason +} + +func validateUserAndGroupAttributes(upstream *v1alpha1.GitHubIdentityProvider) (v1alpha1.GitHubGroupNameAttribute, v1alpha1.GitHubUsernameAttribute, error) { + var groupNameAttribute v1alpha1.GitHubGroupNameAttribute + var usernameAttribute v1alpha1.GitHubUsernameAttribute + + if upstream.Spec.Claims.Groups == nil || upstream.Spec.Claims.Username == nil { + return "", "", fmt.Errorf("spec.claims.groups and spec.claims.username are required") + } else { + groupNameAttribute = *upstream.Spec.Claims.Groups + usernameAttribute = *upstream.Spec.Claims.Username + } + + switch usernameAttribute { + case v1alpha1.GitHubUsernameLoginAndID: + case v1alpha1.GitHubUsernameLogin: + case v1alpha1.GitHubUsernameID: + default: + // Should not happen due to CRD enum validation + return "", "", fmt.Errorf("invalid spec.claims.username (%q)", usernameAttribute) + } + + switch groupNameAttribute { + case v1alpha1.GitHubUseTeamNameForGroupName: + case v1alpha1.GitHubUseTeamSlugForGroupName: + default: + // Should not happen due to CRD enum validation + return "", "", fmt.Errorf("invalid spec.claims.groups (%q)", groupNameAttribute) + } + + return groupNameAttribute, usernameAttribute, nil } func (c *gitHubWatcherController) updateStatus( ctx context.Context, upstream *v1alpha1.GitHubIdentityProvider, - conditions []*metav1.Condition) error { + conditions []*metav1.Condition) (bool, error) { log := c.log.WithValues("namespace", upstream.Namespace, "name", upstream.Name) updated := upstream.DeepCopy() - hadErrorCondition := conditionsutil.MergeIDPConditions(conditions, upstream.Generation, &updated.Status.Conditions, log) + hadErrorCondition := conditionsutil.MergeConditions( + conditions, + upstream.Generation, + &updated.Status.Conditions, + log, + metav1.NewTime(c.clock.Now()), + ) updated.Status.Phase = v1alpha1.GitHubPhaseReady if hadErrorCondition { updated.Status.Phase = v1alpha1.GitHubPhaseError } - if equality.Semantic.DeepEqual(upstream, updated) { - return nil + if equality.Semantic.DeepEqual(upstream, updated) { // untested + return hadErrorCondition, nil } - _, err := c.client. + log.Info("updating GitHubIdentityProvider status", "phase", updated.Status.Phase) + + _, updateStatusError := c.client. IDPV1alpha1(). GitHubIdentityProviders(upstream.Namespace). UpdateStatus(ctx, updated, metav1.UpdateOptions{}) - return err + return hadErrorCondition, updateStatusError } diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go index c9cc9e4cd..0f18cab9e 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go @@ -6,74 +6,1576 @@ package githubupstreamwatcher import ( "bytes" "context" + "crypto/x509" + "encoding/base64" + "fmt" + "net" + "net/http" + "net/http/httptest" + "strings" "testing" + "time" "github.com/stretchr/testify/require" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "golang.org/x/oauth2" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" k8sinformers "k8s.io/client-go/informers" kubernetesfake "k8s.io/client-go/kubernetes/fake" + coretesting "k8s.io/client-go/testing" + "k8s.io/client-go/util/cert" + "k8s.io/utils/clock" + clocktesting "k8s.io/utils/clock/testing" "k8s.io/utils/ptr" "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" pinnipedfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake" pinnipedinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions" + "go.pinniped.dev/internal/certauthority" + "go.pinniped.dev/internal/controller/supervisorconfig/upstreamwatchers" "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/crypto/ptls" "go.pinniped.dev/internal/federationdomain/dynamicupstreamprovider" "go.pinniped.dev/internal/federationdomain/upstreamprovider" + "go.pinniped.dev/internal/net/phttp" "go.pinniped.dev/internal/plog" - "go.pinniped.dev/internal/testutil/githubtestutil" + "go.pinniped.dev/internal/testutil" + "go.pinniped.dev/internal/testutil/tlsserver" "go.pinniped.dev/internal/upstreamgithub" ) -func TestController(t *testing.T) { - t.Parallel() +const countExpectedConditions = 5 - testNamespace := "foo" - testName := "bar" +var ( + githubIDPGVR = schema.GroupVersionResource{ + Group: v1alpha1.SchemeGroupVersion.Group, + Version: v1alpha1.SchemeGroupVersion.Version, + Resource: "githubidentityproviders", + } + + githubIDPKind = v1alpha1.SchemeGroupVersion.WithKind("GitHubIdentityProvider") +) + +func TestController(t *testing.T) { + require.Equal(t, 5, countExpectedConditions) + + goodServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + }), tlsserver.RecordTLSHello) + goodServerDomain, _ := strings.CutPrefix(goodServer.URL, "https://") + goodServerCAB64 := base64.StdEncoding.EncodeToString(tlsserver.TLSTestServerCA(goodServer)) + + goodServerIPv6 := tlsserver.TLSTestIPv6Server(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + }), tlsserver.RecordTLSHello) + goodServerIPv6Domain, _ := strings.CutPrefix(goodServerIPv6.URL, "https://") + goodServerIPv6CAB64 := base64.StdEncoding.EncodeToString(tlsserver.TLSTestServerCA(goodServerIPv6)) + + caForUnknownServer, err := certauthority.New("Some Unknown CA", time.Hour) + require.NoError(t, err) + unknownServerCABytes, _, err := caForUnknownServer.IssueServerCertPEM( + []string{"some-dns-name", "some-other-dns-name"}, + []net.IP{net.ParseIP("10.2.3.4")}, + time.Hour, + ) + require.NoError(t, err) + + generation := int64(1234) + lastTransitionTime := metav1.Time{Time: time.Now().Add(-1 * time.Hour)} + namespace := "some-namespace" + + nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local) + frozenClock := clocktesting.NewFakeClock(nowDoesntMatter) + + goodSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-secret-name", + Namespace: namespace, + }, + Type: "secrets.pinniped.dev/github-client", + Data: map[string][]byte{ + "clientID": []byte("some-client-id"), + "clientSecret": []byte("some-client-secret"), + }, + } + + validMinimalIDP := &v1alpha1.GitHubIdentityProvider{ + ObjectMeta: metav1.ObjectMeta{ + Name: "minimal-idp-name", + Namespace: namespace, + UID: types.UID("minimal-uid"), + Generation: generation, + }, + Spec: v1alpha1.GitHubIdentityProviderSpec{ + GitHubAPI: v1alpha1.GitHubAPIConfig{ + Host: ptr.To(goodServerDomain), + TLS: &v1alpha1.TLSSpec{ + CertificateAuthorityData: goodServerCAB64, + }, + }, + Client: v1alpha1.GitHubClientSpec{ + SecretName: goodSecret.Name, + }, + Claims: v1alpha1.GitHubClaims{ + Username: ptr.To(v1alpha1.GitHubUsernameLogin), + Groups: ptr.To(v1alpha1.GitHubUseTeamSlugForGroupName), + }, + AllowAuthentication: v1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: v1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(v1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers), + }, + }, + }, + } + + validFilledOutIDP := &v1alpha1.GitHubIdentityProvider{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-idp-name", + Namespace: namespace, + UID: types.UID("some-resource-uid"), + Generation: generation, + }, + Spec: v1alpha1.GitHubIdentityProviderSpec{ + GitHubAPI: v1alpha1.GitHubAPIConfig{ + Host: ptr.To(goodServerDomain), + TLS: &v1alpha1.TLSSpec{ + CertificateAuthorityData: goodServerCAB64, + }, + }, + Claims: v1alpha1.GitHubClaims{ + Username: ptr.To(v1alpha1.GitHubUsernameID), + Groups: ptr.To(v1alpha1.GitHubUseTeamNameForGroupName), + }, + AllowAuthentication: v1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: v1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(v1alpha1.GitHubAllowedAuthOrganizationsPolicyOnlyUsersFromAllowedOrganizations), + Allowed: []string{"organization1", "org2"}, + }, + }, + Client: v1alpha1.GitHubClientSpec{ + SecretName: goodSecret.Name, + }, + }, + } + + buildHostValidTrue := func(t *testing.T, host string) metav1.Condition { + t.Helper() + + return metav1.Condition{ + Type: HostValid, + Status: metav1.ConditionTrue, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf("spec.githubAPI.host (%q) is valid", host), + } + } + + buildHostValidFalse := func(t *testing.T, host, message string) metav1.Condition { + t.Helper() + + return metav1.Condition{ + Type: HostValid, + Status: metav1.ConditionFalse, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: "InvalidHost", + Message: fmt.Sprintf(`spec.githubAPI.host (%q) is not valid: %s`, host, message), + } + } + + buildTLSConfigurationValidTrue := func(t *testing.T) metav1.Condition { + t.Helper() + + return metav1.Condition{ + Type: TLSConfigurationValid, + Status: metav1.ConditionTrue, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: upstreamwatchers.ReasonSuccess, + Message: "spec.githubAPI.tls.certificateAuthorityData is valid", + } + } + + buildTLSConfigurationValidFalse := func(t *testing.T, message string) metav1.Condition { + t.Helper() + + return metav1.Condition{ + Type: TLSConfigurationValid, + Status: metav1.ConditionFalse, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: "InvalidTLSConfig", + Message: message, + } + } + + buildOrganizationsPolicyValidTrue := func(t *testing.T, policy v1alpha1.GitHubAllowedAuthOrganizationsPolicy) metav1.Condition { + t.Helper() + + return metav1.Condition{ + Type: OrganizationsPolicyValid, + Status: metav1.ConditionTrue, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf("spec.allowAuthentication.organizations.policy (%q) is valid", policy), + } + } + + buildOrganizationsPolicyValidFalse := func(t *testing.T, message string) metav1.Condition { + t.Helper() + + return metav1.Condition{ + Type: OrganizationsPolicyValid, + Status: metav1.ConditionFalse, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: "Invalid", + Message: message, + } + } + + buildClientCredentialsObtainedTrue := func(t *testing.T, secretName string) metav1.Condition { + t.Helper() + return metav1.Condition{ + Type: ClientCredentialsObtained, + Status: metav1.ConditionTrue, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf("clientID and clientSecret have been read from spec.client.SecretName (%q)", secretName), + } + } + + buildClientCredentialsObtainedFalse := func(t *testing.T, prefix, secretName, namespace, reason string) metav1.Condition { + t.Helper() + + return metav1.Condition{ + Type: ClientCredentialsObtained, + Status: metav1.ConditionFalse, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: reason, + Message: fmt.Sprintf( + `%s: secret from spec.client.SecretName (%q) must be found in namespace %q with type "secrets.pinniped.dev/github-client" and keys "clientID" and "clientSecret"`, + prefix, + secretName, + namespace, + ), + } + } + + buildGitHubConnectionValidTrue := func(t *testing.T, host string) metav1.Condition { + t.Helper() + + return metav1.Condition{ + Type: GitHubConnectionValid, + Status: metav1.ConditionTrue, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf("spec.githubAPI.host (%q) is reachable and TLS verification succeeds", host), + } + } + + buildGitHubConnectionValidFalse := func(t *testing.T, message string) metav1.Condition { + t.Helper() + + return metav1.Condition{ + Type: GitHubConnectionValid, + Status: metav1.ConditionFalse, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: "UnableToDialServer", + Message: message, + } + } + + buildGitHubConnectionValidUnknown := func(t *testing.T) metav1.Condition { + t.Helper() + + return metav1.Condition{ + Type: GitHubConnectionValid, + Status: metav1.ConditionUnknown, + ObservedGeneration: generation, + LastTransitionTime: lastTransitionTime, + Reason: "UnableToValidate", + Message: "unable to validate; see other conditions for details", + } + } + + buildLogForUpdatingClientCredentialsObtained := func(name, status, reason, message string) string { + return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"ClientCredentialsObtained","status":"%s","reason":"%s","message":"%s"}`, name, status, reason, message) + } + + buildLogForUpdatingOrganizationPolicyValid := func(name, status, reason, message string) string { + return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"OrganizationsPolicyValid","status":"%s","reason":"%s","message":"%s"}`, name, status, reason, message) + } + + buildLogForUpdatingHostValid := func(name, status, reason, messageFmt, host string) string { + return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"HostValid","status":"%s","reason":"%s","message":"%s"}`, name, status, reason, fmt.Sprintf(messageFmt, host)) + } + + buildLogForUpdatingTLSConfigurationValid := func(name, status, reason, message string) string { + return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"TLSConfigurationValid","status":"%s","reason":"%s","message":"%s"}`, name, status, reason, message) + } + + buildLogForUpdatingGitHubConnectionValid := func(name, status, reason, messageFmt, host string) string { + return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"GitHubConnectionValid","status":"%s","reason":"%s","message":"%s"}`, name, status, reason, fmt.Sprintf(messageFmt, host)) + } + + buildLogForUpdatingGitHubConnectionValidUnknown := func(name string) string { + return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"conditionsutil/conditions_util.go:$conditionsutil.MergeConditions","message":"updated condition","namespace":"some-namespace","name":"%s","type":"GitHubConnectionValid","status":"Unknown","reason":"UnableToValidate","message":"unable to validate; see other conditions for details"}`, name) + } + + buildLogForUpdatingPhase := func(name, phase string) string { + return fmt.Sprintf(`{"level":"info","timestamp":"2099-08-08T13:57:36.123456Z","logger":"github-upstream-observer","caller":"githubupstreamwatcher/github_upstream_watcher.go:$githubupstreamwatcher.(*gitHubWatcherController).updateStatus","message":"updating GitHubIdentityProvider status","namespace":"some-namespace","name":"%s","phase":"%s"}`, name, phase) + } tests := []struct { name string githubIdentityProviders []runtime.Object - inputSecrets []runtime.Object - configClient func(*pinnipedfake.Clientset) + secrets []runtime.Object wantErr string wantLogs []string - wantResultingCache []*githubtestutil.TestUpstreamGithubIdentityProvider + wantResultingCache []*upstreamgithub.ProviderConfig wantResultingUpstreams []v1alpha1.GitHubIdentityProvider }{ { - name: "no upstreams", - }, { - name: "found github idp is cached", - inputSecrets: []runtime.Object{}, - githubIdentityProviders: []runtime.Object{&v1alpha1.GitHubIdentityProvider{ - ObjectMeta: v1.ObjectMeta{Namespace: testNamespace, Name: testName}, - Spec: v1alpha1.GitHubIdentityProviderSpec{ - GitHubAPI: v1alpha1.GitHubAPIConfig{ - Host: ptr.To("127.0.0.1"), - TLS: &v1alpha1.TLSSpec{ - CertificateAuthorityData: "irrelevant", + name: "no GitHubIdentityProviders", + }, + { + name: "happy path with all fields", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + validFilledOutIDP, + }, + wantResultingCache: []*upstreamgithub.ProviderConfig{ + { + Name: "some-idp-name", + ResourceUID: "some-resource-uid", + Host: fmt.Sprintf("https://%s", *validFilledOutIDP.Spec.GitHubAPI.Host), + UsernameAttribute: "id", + GroupNameAttribute: "name", + OAuth2Config: &oauth2.Config{ + ClientID: "some-client-id", + ClientSecret: "some-client-secret", + }, + AllowedOrganizations: []string{"organization1", "org2"}, + OrganizationLoginPolicy: "OnlyUsersFromAllowedOrganizations", + AuthorizationURL: fmt.Sprintf("https://%s/login/oauth/authorize", *validFilledOutIDP.Spec.GitHubAPI.Host), + HttpClient: buildPretendHttpClient(t, goodServer), + }, + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: validFilledOutIDP.Spec, + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseReady, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), }, }, - Claims: v1alpha1.GitHubClaims{ - Username: ptr.To(v1alpha1.GitHubUsernameID), - Groups: ptr.To(v1alpha1.GitHubUseTeamNameForGroupName), - }, - AllowAuthentication: v1alpha1.GitHubAllowAuthenticationSpec{ - Organizations: v1alpha1.GitHubOrganizationsSpec{ - Policy: ptr.To(v1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers), - Allowed: []string{"foo", "bar"}, - }, - }, - Client: v1alpha1.GitHubClientSpec{ - SecretName: "some-secret", - }, }, - }}, - wantResultingCache: []*githubtestutil.TestUpstreamGithubIdentityProvider{{ - Name: "test-github-idp-to-flesh-out", - }}, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("some-idp-name", "Ready"), + }, + }, + { + name: "happy path with minimal fields", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + validMinimalIDP, + }, + wantResultingCache: []*upstreamgithub.ProviderConfig{ + { + Name: "minimal-idp-name", + ResourceUID: "minimal-uid", + Host: fmt.Sprintf("https://%s", goodServerDomain), + UsernameAttribute: "login", + GroupNameAttribute: "slug", + OAuth2Config: &oauth2.Config{ + ClientID: "some-client-id", + ClientSecret: "some-client-secret", + }, + OrganizationLoginPolicy: "AllGitHubUsers", + AuthorizationURL: fmt.Sprintf("https://%s/login/oauth/authorize", goodServerDomain), + HttpClient: buildPretendHttpClient(t, goodServer), + }, + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: validMinimalIDP.Spec, + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseReady, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), + buildGitHubConnectionValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validMinimalIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validMinimalIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("minimal-idp-name", "Ready"), + }, + }, + { + name: "happy path with IPv6", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + ipv6IDP := validMinimalIDP.DeepCopy() + ipv6IDP.Spec.GitHubAPI.Host = ptr.To(goodServerIPv6Domain) + ipv6IDP.Spec.GitHubAPI.TLS = &v1alpha1.TLSSpec{ + CertificateAuthorityData: goodServerIPv6CAB64, + } + return ipv6IDP + }(), + }, + wantResultingCache: []*upstreamgithub.ProviderConfig{ + { + Name: "minimal-idp-name", + ResourceUID: "minimal-uid", + Host: fmt.Sprintf("https://%s", goodServerIPv6Domain), + UsernameAttribute: "login", + GroupNameAttribute: "slug", + OAuth2Config: &oauth2.Config{ + ClientID: "some-client-id", + ClientSecret: "some-client-secret", + }, + OrganizationLoginPolicy: "AllGitHubUsers", + AuthorizationURL: fmt.Sprintf("https://%s/login/oauth/authorize", goodServerIPv6Domain), + HttpClient: buildPretendHttpClient(t, goodServer), + }, + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + otherSpec := validMinimalIDP.Spec.DeepCopy() + otherSpec.GitHubAPI.Host = ptr.To(goodServerIPv6Domain) + otherSpec.GitHubAPI.TLS = &v1alpha1.TLSSpec{ + CertificateAuthorityData: goodServerIPv6CAB64, + } + return *otherSpec + }(), + + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseReady, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), + buildGitHubConnectionValidTrue(t, goodServerIPv6Domain), + buildHostValidTrue(t, goodServerIPv6Domain), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, goodServerIPv6Domain), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, goodServerIPv6Domain), + buildLogForUpdatingPhase("minimal-idp-name", "Ready"), + }, + }, + { + name: "multiple idps - two good, one invalid", + secrets: []runtime.Object{ + goodSecret, + func() runtime.Object { + otherSecret := goodSecret.DeepCopy() + otherSecret.Name = "other-secret-name" + otherSecret.Data["clientID"] = []byte("other-client-id") + otherSecret.Data["clientSecret"] = []byte("other-client-secret") + return otherSecret + }(), + }, + githubIdentityProviders: []runtime.Object{ + validFilledOutIDP, + func() runtime.Object { + otherIDP := validFilledOutIDP.DeepCopy() + otherIDP.Name = "other-idp-name" + otherIDP.Spec.Client.SecretName = "other-secret-name" + + // No other test happens to that this particular value passes validation + otherIDP.Spec.Claims.Username = ptr.To(v1alpha1.GitHubUsernameLoginAndID) + return otherIDP + }(), + func() runtime.Object { + invalidIDP := validFilledOutIDP.DeepCopy() + invalidIDP.Name = "invalid-idp-name" + invalidIDP.Spec.Client.SecretName = "no-secret-with-this-name" + return invalidIDP + }(), + }, + wantResultingCache: []*upstreamgithub.ProviderConfig{ + { + Name: "some-idp-name", + ResourceUID: "some-resource-uid", + Host: fmt.Sprintf("https://%s", *validFilledOutIDP.Spec.GitHubAPI.Host), + UsernameAttribute: "id", + GroupNameAttribute: "name", + OAuth2Config: &oauth2.Config{ + ClientID: "some-client-id", + ClientSecret: "some-client-secret", + }, + AllowedOrganizations: []string{"organization1", "org2"}, + OrganizationLoginPolicy: "OnlyUsersFromAllowedOrganizations", + AuthorizationURL: fmt.Sprintf("https://%s/login/oauth/authorize", *validFilledOutIDP.Spec.GitHubAPI.Host), + HttpClient: buildPretendHttpClient(t, goodServer), + }, + { + Name: "other-idp-name", + ResourceUID: "some-resource-uid", + Host: fmt.Sprintf("https://%s", *validFilledOutIDP.Spec.GitHubAPI.Host), + UsernameAttribute: "login:id", + GroupNameAttribute: "name", + OAuth2Config: &oauth2.Config{ + ClientID: "other-client-id", + ClientSecret: "other-client-secret", + }, + AllowedOrganizations: []string{"organization1", "org2"}, + OrganizationLoginPolicy: "OnlyUsersFromAllowedOrganizations", + AuthorizationURL: fmt.Sprintf("https://%s/login/oauth/authorize", *validFilledOutIDP.Spec.GitHubAPI.Host), + HttpClient: buildPretendHttpClient(t, goodServer), + }, + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: func() metav1.ObjectMeta { + otherMeta := validFilledOutIDP.ObjectMeta.DeepCopy() + otherMeta.Name = "invalid-idp-name" + return *otherMeta + }(), + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + otherSpec := validFilledOutIDP.Spec.DeepCopy() + otherSpec.Client.SecretName = "no-secret-with-this-name" + return *otherSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedFalse( + t, + `secret "no-secret-with-this-name" not found`, + "no-secret-with-this-name", + namespace, + upstreamwatchers.ReasonNotFound, + ), + buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + { + ObjectMeta: func() metav1.ObjectMeta { + otherMeta := validFilledOutIDP.ObjectMeta.DeepCopy() + otherMeta.Name = "other-idp-name" + return *otherMeta + }(), + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + otherSpec := validFilledOutIDP.Spec.DeepCopy() + otherSpec.Client.SecretName = "other-secret-name" + otherSpec.Claims.Username = ptr.To(v1alpha1.GitHubUsernameLoginAndID) + return *otherSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseReady, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, "other-secret-name"), + buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: validFilledOutIDP.Spec, + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseReady, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("invalid-idp-name", "False", "SecretNotFound", `secret \"no-secret-with-this-name\" not found: secret from spec.client.SecretName (\"no-secret-with-this-name\") must be found in namespace \"some-namespace\" with type \"secrets.pinniped.dev/github-client\" and keys \"clientID\" and \"clientSecret\"`), + buildLogForUpdatingOrganizationPolicyValid("invalid-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("invalid-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("invalid-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("invalid-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("invalid-idp-name", "Error"), + + buildLogForUpdatingClientCredentialsObtained("other-idp-name", "True", "Success", `clientID and clientSecret have been read from spec.client.SecretName (\"other-secret-name\")`), + buildLogForUpdatingOrganizationPolicyValid("other-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("other-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("other-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("other-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("other-idp-name", "Ready"), + + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("some-idp-name", "Ready"), + }, + }, + { + name: "Host error - missing spec.githubAPI.host", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validFilledOutIDP.DeepCopy() + badIDP.Spec.GitHubAPI.Host = nil + return badIDP + }(), + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validFilledOutIDP.Spec.DeepCopy() + badSpec.GitHubAPI.Host = nil + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidUnknown(t), + buildHostValidFalse(t, "", "must not be empty"), + buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("some-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"%s\") is not valid: must not be empty`, ""), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValidUnknown("some-idp-name"), + buildLogForUpdatingPhase("some-idp-name", "Error"), + }, + }, + { + name: "Host error - protocol/schema is specified", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validMinimalIDP.DeepCopy() + badIDP.Spec.GitHubAPI.Host = ptr.To("https://example.com") + return badIDP + }(), + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validMinimalIDP.Spec.DeepCopy() + badSpec.GitHubAPI.Host = ptr.To("https://example.com") + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidUnknown(t), + buildHostValidFalse(t, "https://example.com", `invalid port "//example.com"`), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"%s\") is not valid: invalid port \"//example.com\"`, "https://example.com"), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValidUnknown("minimal-idp-name"), + buildLogForUpdatingPhase("minimal-idp-name", "Error"), + }, + }, + { + name: "Host error - path is specified", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validMinimalIDP.DeepCopy() + badIDP.Spec.GitHubAPI.Host = ptr.To("example.com/foo") + return badIDP + }(), + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validMinimalIDP.Spec.DeepCopy() + badSpec.GitHubAPI.Host = ptr.To("example.com/foo") + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), + buildGitHubConnectionValidUnknown(t), + buildHostValidFalse(t, "example.com/foo", `host "example.com/foo" is not a valid hostname or IP address`), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"%s\") is not valid: host \"example.com/foo\" is not a valid hostname or IP address`, "example.com/foo"), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValidUnknown("minimal-idp-name"), + buildLogForUpdatingPhase("minimal-idp-name", "Error"), + }, + }, + { + name: "Host error - userinfo is specified", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validMinimalIDP.DeepCopy() + badIDP.Spec.GitHubAPI.Host = ptr.To("u:p@example.com") + return badIDP + }(), + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validMinimalIDP.Spec.DeepCopy() + badSpec.GitHubAPI.Host = ptr.To("u:p@example.com") + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), + buildGitHubConnectionValidUnknown(t), + buildHostValidFalse(t, "u:p@example.com", `invalid port "p@example.com"`), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"%s\") is not valid: invalid port \"p@example.com\"`, "u:p@example.com"), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValidUnknown("minimal-idp-name"), + buildLogForUpdatingPhase("minimal-idp-name", "Error"), + }, + }, + { + name: "Host error - query is specified", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validMinimalIDP.DeepCopy() + badIDP.Spec.GitHubAPI.Host = ptr.To("example.com?a=b") + return badIDP + }(), + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validMinimalIDP.Spec.DeepCopy() + badSpec.GitHubAPI.Host = ptr.To("example.com?a=b") + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), + buildGitHubConnectionValidUnknown(t), + buildHostValidFalse(t, "example.com?a=b", `host "example.com?a=b" is not a valid hostname or IP address`), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"%s\") is not valid: host \"example.com?a=b\" is not a valid hostname or IP address`, "example.com?a=b"), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValidUnknown("minimal-idp-name"), + buildLogForUpdatingPhase("minimal-idp-name", "Error"), + }, + }, + { + name: "Host error - fragment is specified", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validMinimalIDP.DeepCopy() + badIDP.Spec.GitHubAPI.Host = ptr.To("example.com#a") + return badIDP + }(), + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validMinimalIDP.Spec.DeepCopy() + badSpec.GitHubAPI.Host = ptr.To("example.com#a") + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), + buildGitHubConnectionValidUnknown(t), + buildHostValidFalse(t, "example.com#a", `host "example.com#a" is not a valid hostname or IP address`), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "False", "InvalidHost", `spec.githubAPI.host (\"%s\") is not valid: host \"example.com#a\" is not a valid hostname or IP address`, "example.com#a"), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValidUnknown("minimal-idp-name"), + buildLogForUpdatingPhase("minimal-idp-name", "Error"), + }, + }, + { + name: "TLS error - invalid bundle", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validFilledOutIDP.DeepCopy() + badIDP.Spec.GitHubAPI.TLS = &v1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte("foo")), + } + return badIDP + }(), + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validFilledOutIDP.Spec.DeepCopy() + badSpec.GitHubAPI.TLS = &v1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte("foo")), + } + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidUnknown(t), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidFalse(t, "spec.githubAPI.tls.certificateAuthorityData is not valid: certificateAuthorityData is not valid PEM: data does not contain any valid RSA or ECDSA certificates"), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "False", "InvalidTLSConfig", "spec.githubAPI.tls.certificateAuthorityData is not valid: certificateAuthorityData is not valid PEM: data does not contain any valid RSA or ECDSA certificates"), + buildLogForUpdatingGitHubConnectionValidUnknown("some-idp-name"), + buildLogForUpdatingPhase("some-idp-name", "Error"), + }, + }, + { + name: "Connection error - no such host", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validMinimalIDP.DeepCopy() + badIDP.Spec.GitHubAPI.Host = ptr.To("nowhere.bad-tld") + return badIDP + }(), + }, + wantErr: "dial tcp: lookup nowhere.bad-tld: no such host", + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validMinimalIDP.Spec.DeepCopy() + badSpec.GitHubAPI.Host = ptr.To("nowhere.bad-tld") + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validMinimalIDP.Spec.Client.SecretName), + buildGitHubConnectionValidFalse(t, fmt.Sprintf(`cannot dial server spec.githubAPI.host (%q): dial tcp: lookup nowhere.bad-tld: no such host`, "nowhere.bad-tld:443")), + buildHostValidTrue(t, "nowhere.bad-tld"), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validMinimalIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, "nowhere.bad-tld"), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "False", "UnableToDialServer", `cannot dial server spec.githubAPI.host (\"%s\"): dial tcp: lookup nowhere.bad-tld: no such host`, "nowhere.bad-tld:443"), + buildLogForUpdatingPhase("minimal-idp-name", "Error"), + }, + }, + { + name: "Connection error - host not trusted by system certs", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validFilledOutIDP.DeepCopy() + badIDP.Spec.GitHubAPI.TLS = nil + return badIDP + }(), + }, + wantErr: "tls: failed to verify certificate: x509: certificate signed by unknown authority", + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validFilledOutIDP.Spec.DeepCopy() + badSpec.GitHubAPI.TLS = nil + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidFalse(t, fmt.Sprintf(`cannot dial server spec.githubAPI.host (%q): tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host)), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "False", "UnableToDialServer", `cannot dial server spec.githubAPI.host (\"%s\"): tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("some-idp-name", "Error"), + }, + }, + { + name: "Connection error - host not trusted by provided CA bundle", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validFilledOutIDP.DeepCopy() + badIDP.Spec.GitHubAPI.TLS = &v1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString(unknownServerCABytes), + } + return badIDP + }(), + }, + wantErr: "tls: failed to verify certificate: x509: certificate signed by unknown authority", + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validFilledOutIDP.Spec.DeepCopy() + badSpec.GitHubAPI.TLS = &v1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString(unknownServerCABytes), + } + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidFalse(t, fmt.Sprintf(`cannot dial server spec.githubAPI.host (%q): tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host)), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "False", "UnableToDialServer", `cannot dial server spec.githubAPI.host (\"%s\"): tls: failed to verify certificate: x509: certificate signed by unknown authority`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("some-idp-name", "Error"), + }, + }, + { + name: "Organization Policy error - missing spec.allowAuthentication.organizations.policy", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validFilledOutIDP.DeepCopy() + badIDP.Spec.AllowAuthentication.Organizations.Policy = nil + return badIDP + }(), + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validFilledOutIDP.Spec.DeepCopy() + badSpec.AllowAuthentication.Organizations.Policy = nil + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidFalse(t, "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed"), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "False", "Invalid", "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed"), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("some-idp-name", "Error"), + }, + }, + { + name: "Organization Policy error - invalid spec.allowAuthentication.organizations.policy", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validFilledOutIDP.DeepCopy() + badIDP.Spec.AllowAuthentication.Organizations.Policy = ptr.To[v1alpha1.GitHubAllowedAuthOrganizationsPolicy]("a") + return badIDP + }(), + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validFilledOutIDP.Spec.DeepCopy() + badSpec.AllowAuthentication.Organizations.Policy = ptr.To[v1alpha1.GitHubAllowedAuthOrganizationsPolicy]("a") + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidFalse(t, "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed"), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "False", "Invalid", "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed"), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("some-idp-name", "Error"), + }, + }, + { + name: "Organization Policy error - spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validFilledOutIDP.DeepCopy() + badIDP.Spec.AllowAuthentication.Organizations.Policy = ptr.To(v1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers) + return badIDP + }(), + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validFilledOutIDP.Spec.DeepCopy() + badSpec.AllowAuthentication.Organizations.Policy = ptr.To(v1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers) + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidFalse(t, "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed"), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "False", "Invalid", "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed"), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("some-idp-name", "Error"), + }, + }, + { + name: "Organization Policy error - spec.allowAuthentication.organizations.policy must be 'AllGitHubUsers' when spec.allowAuthentication.organizations.allowed is empty", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validFilledOutIDP.DeepCopy() + badIDP.Spec.AllowAuthentication.Organizations.Allowed = nil + return badIDP + }(), + }, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validFilledOutIDP.Spec.DeepCopy() + badSpec.AllowAuthentication.Organizations.Allowed = nil + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidFalse(t, "spec.allowAuthentication.organizations.policy must be 'AllGitHubUsers' when spec.allowAuthentication.organizations.allowed is empty"), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "False", "Invalid", "spec.allowAuthentication.organizations.policy must be 'AllGitHubUsers' when spec.allowAuthentication.organizations.allowed is empty"), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("some-idp-name", "Error"), + }, + }, + { + name: "Invalid Claims - missing spec.claims.username", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validFilledOutIDP.DeepCopy() + badIDP.Spec.Claims.Username = nil + return badIDP + }(), + }, + wantErr: "spec.claims.groups and spec.claims.username are required", + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validFilledOutIDP.Spec.DeepCopy() + badSpec.Claims.Username = nil + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseReady, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("some-idp-name", "Ready"), + }, + }, + { + name: "Invalid Claims - invalid spec.claims.username", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validFilledOutIDP.DeepCopy() + badIDP.Spec.Claims.Username = ptr.To[v1alpha1.GitHubUsernameAttribute]("a") + return badIDP + }(), + }, + wantErr: "invalid spec.claims.username", + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validFilledOutIDP.Spec.DeepCopy() + badSpec.Claims.Username = ptr.To[v1alpha1.GitHubUsernameAttribute]("a") + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseReady, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("some-idp-name", "Ready"), + }, + }, + { + name: "Invalid Claims - missing spec.claims.groups", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validFilledOutIDP.DeepCopy() + badIDP.Spec.Claims.Groups = nil + return badIDP + }(), + }, + wantErr: "spec.claims.groups and spec.claims.username are required", + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validFilledOutIDP.Spec.DeepCopy() + badSpec.Claims.Groups = nil + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseReady, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("some-idp-name", "Ready"), + }, + }, + { + name: "Invalid Claims - invalid spec.claims.groups", + secrets: []runtime.Object{goodSecret}, + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + badIDP := validFilledOutIDP.DeepCopy() + badIDP.Spec.Claims.Groups = ptr.To[v1alpha1.GitHubGroupNameAttribute]("a") + return badIDP + }(), + }, + wantErr: "invalid spec.claims.groups", + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validFilledOutIDP.ObjectMeta, + Spec: func() v1alpha1.GitHubIdentityProviderSpec { + badSpec := validFilledOutIDP.Spec.DeepCopy() + badSpec.Claims.Groups = ptr.To[v1alpha1.GitHubGroupNameAttribute]("a") + return *badSpec + }(), + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseReady, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedTrue(t, validFilledOutIDP.Spec.Client.SecretName), + buildGitHubConnectionValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("some-idp-name", "True", "Success", fmt.Sprintf(`clientID and clientSecret have been read from spec.client.SecretName (\"%s\")`, validFilledOutIDP.Spec.Client.SecretName)), + buildLogForUpdatingOrganizationPolicyValid("some-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validFilledOutIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("some-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("some-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("some-idp-name", "Ready"), + }, + }, + { + name: "Client Secret error - in different namespace", + secrets: []runtime.Object{ + func() runtime.Object { + badSecret := goodSecret.DeepCopy() + badSecret.Namespace = "other-namespace" + return badSecret + }(), + }, + githubIdentityProviders: []runtime.Object{validMinimalIDP}, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: validMinimalIDP.Spec, + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedFalse( + t, + fmt.Sprintf("secret %q not found", validMinimalIDP.Spec.Client.SecretName), + validMinimalIDP.Spec.Client.SecretName, + validMinimalIDP.Namespace, + upstreamwatchers.ReasonNotFound, + ), + buildGitHubConnectionValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "False", "SecretNotFound", `secret \"some-secret-name\" not found: secret from spec.client.SecretName (\"some-secret-name\") must be found in namespace \"some-namespace\" with type \"secrets.pinniped.dev/github-client\" and keys \"clientID\" and \"clientSecret\"`), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("minimal-idp-name", "Error"), + }, + }, + { + name: "Client Secret error - wrong type", + secrets: []runtime.Object{ + func() runtime.Object { + badSecret := goodSecret.DeepCopy() + badSecret.Type = "other-type" + return badSecret + }(), + }, + githubIdentityProviders: []runtime.Object{validMinimalIDP}, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: validMinimalIDP.Spec, + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedFalse( + t, + `wrong secret type "other-type"`, + validMinimalIDP.Spec.Client.SecretName, + validMinimalIDP.Namespace, + upstreamwatchers.ReasonNotFound, + ), + buildGitHubConnectionValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "False", "SecretNotFound", `wrong secret type \"other-type\": secret from spec.client.SecretName (\"some-secret-name\") must be found in namespace \"some-namespace\" with type \"secrets.pinniped.dev/github-client\" and keys \"clientID\" and \"clientSecret\"`), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validMinimalIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validMinimalIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("minimal-idp-name", "Error"), + }, + }, + { + name: "Client Secret error - missing clientId", + secrets: []runtime.Object{ + func() runtime.Object { + badSecret := goodSecret.DeepCopy() + delete(badSecret.Data, "clientID") + return badSecret + }(), + }, + githubIdentityProviders: []runtime.Object{validMinimalIDP}, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: validMinimalIDP.Spec, + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedFalse( + t, + `missing key "clientID"`, + validMinimalIDP.Spec.Client.SecretName, + validMinimalIDP.Namespace, + upstreamwatchers.ReasonNotFound, + ), + buildGitHubConnectionValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "False", "SecretNotFound", `missing key \"clientID\": secret from spec.client.SecretName (\"some-secret-name\") must be found in namespace \"some-namespace\" with type \"secrets.pinniped.dev/github-client\" and keys \"clientID\" and \"clientSecret\"`), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("minimal-idp-name", "Error"), + }, + }, + { + name: "Client Secret error - missing clientSecret", + secrets: []runtime.Object{ + func() runtime.Object { + badSecret := goodSecret.DeepCopy() + delete(badSecret.Data, "clientSecret") + return badSecret + }(), + }, + githubIdentityProviders: []runtime.Object{validMinimalIDP}, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: validMinimalIDP.Spec, + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedFalse( + t, + `missing key "clientSecret"`, + validMinimalIDP.Spec.Client.SecretName, + validMinimalIDP.Namespace, + upstreamwatchers.ReasonNotFound, + ), + buildGitHubConnectionValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "False", "SecretNotFound", `missing key \"clientSecret\": secret from spec.client.SecretName (\"some-secret-name\") must be found in namespace \"some-namespace\" with type \"secrets.pinniped.dev/github-client\" and keys \"clientID\" and \"clientSecret\"`), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validMinimalIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validMinimalIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("minimal-idp-name", "Error"), + }, + }, + { + name: "Client Secret error - additional data", + secrets: []runtime.Object{ + func() runtime.Object { + badSecret := goodSecret.DeepCopy() + badSecret.Data["foo"] = []byte("bar") + return badSecret + }(), + }, + githubIdentityProviders: []runtime.Object{validMinimalIDP}, + wantResultingUpstreams: []v1alpha1.GitHubIdentityProvider{ + { + ObjectMeta: validMinimalIDP.ObjectMeta, + Spec: validMinimalIDP.Spec, + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + buildClientCredentialsObtainedFalse( + t, + "extra keys found", + validMinimalIDP.Spec.Client.SecretName, + validMinimalIDP.Namespace, + upstreamwatchers.ReasonNotFound, + ), + buildGitHubConnectionValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), + buildHostValidTrue(t, *validMinimalIDP.Spec.GitHubAPI.Host), + buildOrganizationsPolicyValidTrue(t, *validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy), + buildTLSConfigurationValidTrue(t), + }, + }, + }, + }, + wantLogs: []string{ + buildLogForUpdatingClientCredentialsObtained("minimal-idp-name", "False", "SecretNotFound", `extra keys found: secret from spec.client.SecretName (\"some-secret-name\") must be found in namespace \"some-namespace\" with type \"secrets.pinniped.dev/github-client\" and keys \"clientID\" and \"clientSecret\"`), + buildLogForUpdatingOrganizationPolicyValid("minimal-idp-name", "True", "Success", fmt.Sprintf(`spec.allowAuthentication.organizations.policy (\"%s\") is valid`, string(*validMinimalIDP.Spec.AllowAuthentication.Organizations.Policy))), + buildLogForUpdatingHostValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is valid`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingTLSConfigurationValid("minimal-idp-name", "True", "Success", "spec.githubAPI.tls.certificateAuthorityData is valid"), + buildLogForUpdatingGitHubConnectionValid("minimal-idp-name", "True", "Success", `spec.githubAPI.host (\"%s\") is reachable and TLS verification succeeds`, *validFilledOutIDP.Spec.GitHubAPI.Host), + buildLogForUpdatingPhase("minimal-idp-name", "Error"), + }, }, } @@ -82,11 +1584,11 @@ func TestController(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - pinnipedAPIClient := pinnipedfake.NewSimpleClientset(tt.githubIdentityProviders...) + fakePinnipedClient := pinnipedfake.NewSimpleClientset(tt.githubIdentityProviders...) fakePinnipedClientForInformers := pinnipedfake.NewSimpleClientset(tt.githubIdentityProviders...) pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(fakePinnipedClientForInformers, 0) - fakeKubeClient := kubernetesfake.NewSimpleClientset(tt.inputSecrets...) + fakeKubeClient := kubernetesfake.NewSimpleClientset(tt.secrets...) kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(fakeKubeClient, 0) cache := dynamicupstreamprovider.NewDynamicUpstreamIDPProvider() @@ -97,13 +1599,18 @@ func TestController(t *testing.T) { var log bytes.Buffer logger := plog.TestLogger(t, &log) + gitHubIdentityProviderInformer := pinnipedInformers.IDP().V1alpha1().GitHubIdentityProviders() + controller := New( + namespace, cache, - pinnipedAPIClient, - pinnipedInformers.IDP().V1alpha1().GitHubIdentityProviders(), + fakePinnipedClient, + gitHubIdentityProviderInformer, kubeInformers.Core().V1().Secrets(), logger, controllerlib.WithInformer, + frozenClock, + buildHttpClient, ) ctx, cancel := context.WithCancel(context.Background()) @@ -115,14 +1622,447 @@ func TestController(t *testing.T) { syncCtx := controllerlib.Context{Context: ctx, Key: controllerlib.Key{}} - if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantErr != "" { - require.EqualError(t, err, tt.wantErr) + if err := controllerlib.TestSync(t, controller, syncCtx); len(tt.wantErr) > 0 { + require.ErrorContains(t, err, controllerlib.ErrSyntheticRequeue.Error()) + require.ErrorContains(t, err, tt.wantErr) } else { require.NoError(t, err) } + // Verify what's in the cache actualIDPList := cache.GetGitHubIdentityProviders() require.Equal(t, len(tt.wantResultingCache), len(actualIDPList)) + for i := 0; i < len(tt.wantResultingCache); i++ { + // Do not expect any particular order in the cache + var actualIDP *upstreamgithub.ProviderConfig + for _, possibleIDP := range actualIDPList { + if possibleIDP.GetName() == tt.wantResultingCache[i].Name { + // For this check, we know that the actual IDPs are going to have type upstreamgithub.ProviderConfig + var ok bool + actualIDP, ok = possibleIDP.(*upstreamgithub.ProviderConfig) + require.True(t, ok) + break + } + } + + require.Equal(t, tt.wantResultingCache[i].Name, actualIDP.GetName()) + require.Equal(t, tt.wantResultingCache[i].ResourceUID, actualIDP.GetResourceUID()) + require.Equal(t, tt.wantResultingCache[i].Host, actualIDP.GetHost()) + require.Equal(t, tt.wantResultingCache[i].OAuth2Config.ClientID, actualIDP.GetClientID()) + require.Equal(t, tt.wantResultingCache[i].GroupNameAttribute, actualIDP.GetGroupNameAttribute()) + require.Equal(t, tt.wantResultingCache[i].UsernameAttribute, actualIDP.GetUsernameAttribute()) + require.Equal(t, tt.wantResultingCache[i].AllowedOrganizations, actualIDP.GetAllowedOrganizations()) + require.Equal(t, tt.wantResultingCache[i].OrganizationLoginPolicy, actualIDP.GetOrganizationLoginPolicy()) + require.Equal(t, tt.wantResultingCache[i].AuthorizationURL, actualIDP.GetAuthorizationURL()) + compareTLSClientConfigWithinHttpClients(t, tt.wantResultingCache[i].HttpClient, actualIDP.GetHttpClient()) + require.Equal(t, tt.wantResultingCache[i].OAuth2Config, actualIDP.OAuth2Config) + } + + // Verify the status conditions as reported in Kubernetes + allGitHubIDPs, err := fakePinnipedClient.IDPV1alpha1().GitHubIdentityProviders(namespace).List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + require.Equal(t, len(tt.wantResultingUpstreams), len(allGitHubIDPs.Items)) + for i := 0; i < len(tt.wantResultingUpstreams); i++ { + // Do not expect any particular order in the K8s objects + var actualIDP *v1alpha1.GitHubIdentityProvider + for _, possibleMatch := range allGitHubIDPs.Items { + if possibleMatch.GetName() == tt.wantResultingUpstreams[i].Name { + actualIDP = ptr.To(possibleMatch) + break + } + } + + require.NotNil(t, actualIDP, "must find IDP with name %s", tt.wantResultingUpstreams[i].Name) + require.Equal(t, countExpectedConditions, len(actualIDP.Status.Conditions)) + + // Update all expected conditions to the frozenTime. + // TODO: Push this out to the test table + for j := 0; j < countExpectedConditions; j++ { + // Get this as a pointer so that we can update the value within the array + condition := &tt.wantResultingUpstreams[i].Status.Conditions[j] + condition.LastTransitionTime = metav1.Time{Time: frozenClock.Now()} + } + + require.Equal(t, tt.wantResultingUpstreams[i], *actualIDP) + } + + expectedLogs := "" + if len(tt.wantLogs) > 0 { + expectedLogs = strings.Join(tt.wantLogs, "\n") + "\n" + } + require.Equal(t, expectedLogs, log.String()) + + // This needs to happen after the expected condition LastTransitionTime has been updated. + wantActions := make([]coretesting.Action, 1+len(tt.wantResultingUpstreams)) + for i, want := range tt.wantResultingUpstreams { + wantActions[i] = coretesting.NewUpdateSubresourceAction(githubIDPGVR, "status", want.Namespace, ptr.To(want)) + } + wantActions[len(tt.wantResultingUpstreams)] = coretesting.NewListAction(githubIDPGVR, githubIDPKind, namespace, metav1.ListOptions{}) + require.Equal(t, wantActions, fakePinnipedClient.Actions()) + }) + } +} + +func TestController_WithExistingConditions(t *testing.T) { + require.Equal(t, 5, countExpectedConditions) + + goodServer := tlsserver.TLSTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + }), tlsserver.RecordTLSHello) + goodServerDomain, _ := strings.CutPrefix(goodServer.URL, "https://") + goodServerCAB64 := base64.StdEncoding.EncodeToString(tlsserver.TLSTestServerCA(goodServer)) + + oneHourAgo := metav1.Time{Time: time.Now().Add(-1 * time.Hour)} + namespace := "existing-conditions-namespace" + + nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local) + frozenClock := clocktesting.NewFakeClock(nowDoesntMatter) + + alreadyInvalidExistingIDP := &v1alpha1.GitHubIdentityProvider{ + ObjectMeta: metav1.ObjectMeta{ + Name: "already-existing-invalid-idp-name", + Namespace: namespace, + UID: types.UID("some-resource-uid"), + Generation: 333, + }, + Spec: v1alpha1.GitHubIdentityProviderSpec{ + GitHubAPI: v1alpha1.GitHubAPIConfig{ + Host: ptr.To(goodServerDomain), + TLS: &v1alpha1.TLSSpec{ + CertificateAuthorityData: goodServerCAB64, + }, + }, + AllowAuthentication: v1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: v1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(v1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers), + }, + }, + Claims: v1alpha1.GitHubClaims{ + Username: ptr.To(v1alpha1.GitHubUsernameLogin), + Groups: ptr.To(v1alpha1.GitHubUseTeamSlugForGroupName), + }, + Client: v1alpha1.GitHubClientSpec{ + SecretName: "unknown-secret", + }, + }, + Status: v1alpha1.GitHubIdentityProviderStatus{ + Phase: v1alpha1.GitHubPhaseError, + Conditions: []metav1.Condition{ + { + Type: ClientCredentialsObtained, + Status: metav1.ConditionFalse, + ObservedGeneration: 333, + LastTransitionTime: oneHourAgo, + Reason: "SecretNotFound", + Message: fmt.Sprintf(`secret "unknown-secret" not found: secret from spec.client.SecretName ("unknown-secret") must be found in namespace %q with type "secrets.pinniped.dev/github-client" and keys "clientID" and "clientSecret"`, namespace), + }, + { + Type: GitHubConnectionValid, + Status: metav1.ConditionTrue, + ObservedGeneration: 333, + LastTransitionTime: oneHourAgo, + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf("spec.githubAPI.host (%q) is reachable and TLS verification succeeds", goodServerDomain), + }, + { + Type: HostValid, + Status: metav1.ConditionTrue, + ObservedGeneration: 333, + LastTransitionTime: oneHourAgo, + Reason: upstreamwatchers.ReasonSuccess, + Message: fmt.Sprintf("spec.githubAPI.host (%q) is valid", goodServerDomain), + }, + { + Type: OrganizationsPolicyValid, + Status: metav1.ConditionTrue, + ObservedGeneration: 333, + LastTransitionTime: oneHourAgo, + Reason: upstreamwatchers.ReasonSuccess, + Message: `spec.allowAuthentication.organizations.policy ("AllGitHubUsers") is valid`, + }, + { + Type: TLSConfigurationValid, + Status: metav1.ConditionTrue, + ObservedGeneration: 333, + LastTransitionTime: oneHourAgo, + Reason: upstreamwatchers.ReasonSuccess, + Message: "spec.githubAPI.tls.certificateAuthorityData is valid", + }, + }, + }, + } + + tests := []struct { + name string + secrets []runtime.Object + githubIdentityProviders []runtime.Object + wantActions []coretesting.Action + }{ + { + name: "no GitHubIdentityProviders", + wantActions: make([]coretesting.Action, 0), + }, + { + name: "already existing idp with appropriate conditions does not issue actions", + githubIdentityProviders: []runtime.Object{ + alreadyInvalidExistingIDP, + }, + wantActions: make([]coretesting.Action, 0), + }, + { + name: "already existing idp with stale conditions will issue an update action", + githubIdentityProviders: []runtime.Object{ + func() runtime.Object { + otherIDP := alreadyInvalidExistingIDP.DeepCopy() + otherIDP.Generation = 400 + otherIDP.Status.Phase = v1alpha1.GitHubPhaseReady + otherIDP.Status.Conditions[0].Status = metav1.ConditionTrue + otherIDP.Status.Conditions[0].Message = "some other message indicating that things are good" + return otherIDP + }(), + }, + wantActions: []coretesting.Action{ + func() coretesting.Action { + idp := alreadyInvalidExistingIDP.DeepCopy() + idp.Generation = 400 + for i := range idp.Status.Conditions { + idp.Status.Conditions[i].ObservedGeneration = 400 + } + idp.Status.Conditions[0].LastTransitionTime = metav1.Time{Time: nowDoesntMatter} + wantAction := coretesting.NewUpdateSubresourceAction(githubIDPGVR, "status", namespace, idp) + return wantAction + }(), + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + fakePinnipedClient := pinnipedfake.NewSimpleClientset(tt.githubIdentityProviders...) + pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(pinnipedfake.NewSimpleClientset(tt.githubIdentityProviders...), 0) + + kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(tt.secrets...), 0) + + var log bytes.Buffer + logger := plog.TestLogger(t, &log) + + controller := New( + namespace, + dynamicupstreamprovider.NewDynamicUpstreamIDPProvider(), + fakePinnipedClient, + pinnipedInformers.IDP().V1alpha1().GitHubIdentityProviders(), + kubeInformers.Core().V1().Secrets(), + logger, + controllerlib.WithInformer, + frozenClock, + buildHttpClient, + ) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + pinnipedInformers.Start(ctx.Done()) + kubeInformers.Start(ctx.Done()) + controllerlib.TestRunSynchronously(t, controller) + + syncCtx := controllerlib.Context{Context: ctx, Key: controllerlib.Key{}} + + err := controllerlib.TestSync(t, controller, syncCtx) + require.NoError(t, err) + + require.Equal(t, tt.wantActions, fakePinnipedClient.Actions()) + }) + } +} + +func buildPretendHttpClient(t *testing.T, server *httptest.Server) *http.Client { + t.Helper() + rootCAs, err := cert.NewPoolFromBytes(tlsserver.TLSTestServerCA(server)) + require.NoError(t, err) + return buildHttpClient(rootCAs) +} + +func buildHttpClient(rootCAs *x509.CertPool) *http.Client { + baseRT := http.DefaultTransport.(*http.Transport).Clone() + baseRT.TLSClientConfig = ptls.Default(rootCAs) + + return &http.Client{ + Transport: baseRT, + } +} + +func compareTLSClientConfigWithinHttpClients(t *testing.T, c1 *http.Client, c2 *http.Client) { + t.Helper() + + if c1 == nil { + require.Nil(t, c2) + return + } + + t1, ok := c1.Transport.(*http.Transport) + require.True(t, ok) + require.NotNil(t, t1) + require.NotNil(t, t1.TLSClientConfig) + + t2, ok := c2.Transport.(*http.Transport) + require.True(t, ok) + require.NotNil(t, t2) + require.NotNil(t, t2.TLSClientConfig) + + require.Equal(t, t1.TLSClientConfig.ClientCAs, t2.TLSClientConfig.ClientCAs) +} + +func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) { + namespace := "some-namespace" + goodSecret := &corev1.Secret{ + Type: "secrets.pinniped.dev/github-client", + ObjectMeta: metav1.ObjectMeta{ + Name: "some-name", + Namespace: namespace, + }, + } + + tests := []struct { + name string + secret metav1.Object + wantAdd bool + wantUpdate bool + wantDelete bool + }{ + { + name: "a secret of the right type", + secret: goodSecret, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "a secret of the right type, but in the wrong namespace", + secret: func() *corev1.Secret { + otherSecret := goodSecret.DeepCopy() + otherSecret.Namespace = "other-namespace" + return otherSecret + }(), + }, + { + name: "a secret of the wrong type", + secret: func() *corev1.Secret { + otherSecret := goodSecret.DeepCopy() + otherSecret.Type = "other-type" + return otherSecret + }(), + }, + { + name: "resource of wrong data type", + secret: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: "some-name", Namespace: "some-namespace"}, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0) + + var log bytes.Buffer + logger := plog.TestLogger(t, &log) + + secretInformer := kubeInformers.Core().V1().Secrets() + observableInformers := testutil.NewObservableWithInformerOption() + + _ = New( + namespace, + dynamicupstreamprovider.NewDynamicUpstreamIDPProvider(), + pinnipedfake.NewSimpleClientset(), + pinnipedinformers.NewSharedInformerFactory(pinnipedfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders(), + secretInformer, + logger, + observableInformers.WithInformer, + clock.RealClock{}, + phttp.Default, + ) + + unrelated := &corev1.Secret{} + filter := observableInformers.GetFilterForInformer(secretInformer) + require.Equal(t, tt.wantAdd, filter.Add(tt.secret)) + require.Equal(t, tt.wantUpdate, filter.Update(unrelated, tt.secret)) + require.Equal(t, tt.wantUpdate, filter.Update(tt.secret, unrelated)) + require.Equal(t, tt.wantDelete, filter.Delete(tt.secret)) + }) + } +} + +func TestGitHubUpstreamWatcherControllerFilterGitHubIDP(t *testing.T) { + namespace := "some-namespace" + goodIDP := &v1alpha1.GitHubIdentityProvider{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + }, + } + + tests := []struct { + name string + idp metav1.Object + wantAdd bool + wantUpdate bool + wantDelete bool + }{ + { + name: "an IDP in the right namespace", + idp: goodIDP, + wantAdd: true, + wantUpdate: true, + wantDelete: true, + }, + { + name: "IDP in the wrong namespace", + idp: func() metav1.Object { + badIDP := goodIDP.DeepCopy() + badIDP.Namespace = "other-namespace" + return badIDP + }(), + }, + { + name: "resource of wrong data type", + idp: &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: "some-name"}, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var log bytes.Buffer + logger := plog.TestLogger(t, &log) + + gitHubIdentityProviderInformer := pinnipedinformers.NewSharedInformerFactory(pinnipedfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders() + observableInformers := testutil.NewObservableWithInformerOption() + + _ = New( + namespace, + dynamicupstreamprovider.NewDynamicUpstreamIDPProvider(), + pinnipedfake.NewSimpleClientset(), + gitHubIdentityProviderInformer, + k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0).Core().V1().Secrets(), + logger, + observableInformers.WithInformer, + clock.RealClock{}, + phttp.Default, + ) + + unrelated := &v1alpha1.GitHubIdentityProvider{} + filter := observableInformers.GetFilterForInformer(gitHubIdentityProviderInformer) + require.Equal(t, tt.wantAdd, filter.Add(tt.idp)) + require.Equal(t, tt.wantUpdate, filter.Update(unrelated, tt.idp)) + require.Equal(t, tt.wantUpdate, filter.Update(tt.idp, unrelated)) + require.Equal(t, tt.wantDelete, filter.Delete(tt.idp)) }) } } diff --git a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go index f8a027144..971dc0ff9 100644 --- a/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go +++ b/internal/controller/supervisorconfig/ldapupstreamwatcher/ldap_upstream_watcher.go @@ -260,7 +260,7 @@ func (c *ldapWatcherController) updateStatus(ctx context.Context, upstream *v1al log := plog.WithValues("namespace", upstream.Namespace, "name", upstream.Name) updated := upstream.DeepCopy() - hadErrorCondition := conditionsutil.MergeIDPConditions(conditions, upstream.Generation, &updated.Status.Conditions, log) + hadErrorCondition := conditionsutil.MergeConditions(conditions, upstream.Generation, &updated.Status.Conditions, log, metav1.Now()) updated.Status.Phase = v1alpha1.LDAPPhaseReady if hadErrorCondition { diff --git a/internal/controller/supervisorconfig/oidcclientwatcher/oidc_client_watcher.go b/internal/controller/supervisorconfig/oidcclientwatcher/oidc_client_watcher.go index 9445fc343..1ab0240d3 100644 --- a/internal/controller/supervisorconfig/oidcclientwatcher/oidc_client_watcher.go +++ b/internal/controller/supervisorconfig/oidcclientwatcher/oidc_client_watcher.go @@ -1,4 +1,4 @@ -// Copyright 2022-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2022-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package oidcclientwatcher @@ -133,7 +133,7 @@ func (c *oidcClientWatcherController) updateStatus( ) error { updated := upstream.DeepCopy() - hadErrorCondition := conditionsutil.MergeConfigConditions(conditions, + hadErrorCondition := conditionsutil.MergeConditions(conditions, upstream.Generation, &updated.Status.Conditions, plog.New(), metav1.Now()) updated.Status.Phase = v1alpha1.OIDCClientPhaseReady diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go index 396df85dc..913371f98 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher.go @@ -412,7 +412,7 @@ func (c *oidcWatcherController) updateStatus(ctx context.Context, upstream *v1al log := c.log.WithValues("namespace", upstream.Namespace, "name", upstream.Name) updated := upstream.DeepCopy() - hadErrorCondition := conditionsutil.MergeIDPConditions(conditions, upstream.Generation, &updated.Status.Conditions, log) + hadErrorCondition := conditionsutil.MergeConditions(conditions, upstream.Generation, &updated.Status.Conditions, log, metav1.Now()) updated.Status.Phase = v1alpha1.PhaseReady if hadErrorCondition { diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 1b938e83b..110b09bee 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -1,12 +1,20 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package controller import ( + "crypto/x509" + "encoding/base64" + "fmt" + "slices" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/util/cert" + authv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1" + idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" "go.pinniped.dev/internal/controllerlib" ) @@ -43,12 +51,16 @@ func SimpleFilter(match func(metav1.Object) bool, parentFunc controllerlib.Paren } } -func MatchAnySecretOfTypeFilter(secretType corev1.SecretType, parentFunc controllerlib.ParentFunc) controllerlib.Filter { +func MatchAnySecretOfTypeFilter(secretType corev1.SecretType, parentFunc controllerlib.ParentFunc, namespaces ...string) controllerlib.Filter { isSecretOfType := func(obj metav1.Object) bool { secret, ok := obj.(*corev1.Secret) if !ok { return false } + // Only match on namespace if namespaces are provided + if len(namespaces) > 0 && !slices.Contains(namespaces, secret.Namespace) { + return false + } return secret.Type == secretType } return SimpleFilter(isSecretOfType, parentFunc) @@ -87,3 +99,43 @@ type WithInformerOptionFunc func( // Same signature as controllerlib.WithInitialEvent(). type WithInitialEventOptionFunc func(key controllerlib.Key) controllerlib.Option + +// BuildCertPoolAuth returns a PEM-encoded CA bundle from the provided spec. If the provided spec is nil, a +// nil CA bundle will be returned. If the provided spec contains a CA bundle that is not properly +// encoded, an error will be returned. +func BuildCertPoolAuth(spec *authv1alpha1.TLSSpec) (*x509.CertPool, []byte, error) { + if spec == nil { + return nil, nil, nil + } + + return buildCertPool(spec.CertificateAuthorityData) +} + +// BuildCertPoolIDP returns a PEM-encoded CA bundle from the provided spec. If the provided spec is nil, a +// nil CA bundle will be returned. If the provided spec contains a CA bundle that is not properly +// encoded, an error will be returned. +func BuildCertPoolIDP(spec *idpv1alpha1.TLSSpec) (*x509.CertPool, []byte, error) { + if spec == nil { + return nil, nil, nil + } + + return buildCertPool(spec.CertificateAuthorityData) +} + +func buildCertPool(certificateAuthorityData string) (*x509.CertPool, []byte, error) { + if len(certificateAuthorityData) == 0 { + return nil, nil, nil + } + + pem, err := base64.StdEncoding.DecodeString(certificateAuthorityData) + if err != nil { + return nil, nil, err + } + + rootCAs, err := cert.NewPoolFromBytes(pem) + if err != nil { + return nil, nil, fmt.Errorf("certificateAuthorityData is not valid PEM: %w", err) + } + + return rootCAs, pem, nil +} diff --git a/internal/endpointaddr/endpointaddr.go b/internal/endpointaddr/endpointaddr.go index c6d0223af..2062af512 100644 --- a/internal/endpointaddr/endpointaddr.go +++ b/internal/endpointaddr/endpointaddr.go @@ -17,7 +17,7 @@ import ( type HostPort struct { // Host is the validated host part of the input, which may be a hostname or IP. // - // This string can be be used as an x509 certificate SAN. + // This string can be used as a x509 certificate SAN. Host string // Port is the validated port number, which may be defaulted. diff --git a/internal/federationdomain/upstreamprovider/upsteam_provider.go b/internal/federationdomain/upstreamprovider/upsteam_provider.go index 5a7c59fa4..ab5920bdf 100644 --- a/internal/federationdomain/upstreamprovider/upsteam_provider.go +++ b/internal/federationdomain/upstreamprovider/upsteam_provider.go @@ -5,11 +5,13 @@ package upstreamprovider import ( "context" + "net/http" "net/url" "golang.org/x/oauth2" "k8s.io/apimachinery/pkg/types" + "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" "go.pinniped.dev/internal/authenticators" "go.pinniped.dev/pkg/oidcclient/nonce" "go.pinniped.dev/pkg/oidcclient/oidctypes" @@ -128,4 +130,40 @@ type UpstreamLDAPIdentityProviderI interface { type UpstreamGithubIdentityProviderI interface { UpstreamIdentityProviderI + + // GetHost returns the hostname of the GitHub server. This is either "github.com" or a GitHub Enterprise Server. + GetHost() string + + // GetClientID returns the OAuth client ID registered with the upstream provider to be used in the authorization code flow. + GetClientID() string + + // GetUsernameAttribute returns the attribute from the GitHub API user response to use for the downstream username. + // See https://docs.github.com/en/rest/users/users?apiVersion=2022-11-28#get-the-authenticated-user. + // Note that this is a constructed value - do not expect that the result will exactly match one of the JSON fields. + GetUsernameAttribute() v1alpha1.GitHubUsernameAttribute + + // GetGroupNameAttribute returns the attribute from the GitHub API team response to use for the downstream group names. + // See https://docs.github.com/en/rest/teams/teams?apiVersion=2022-11-28#list-teams-for-the-authenticated-user. + // Note that this is a constructed value - do not expect that the result will exactly match one of the JSON fields. + GetGroupNameAttribute() v1alpha1.GitHubGroupNameAttribute + + // GetAllowedOrganizations returns a list of organizations configured to allow authentication. A user must have membership + // in at least one of these organizations to log in. Note that the user can specify a policy (returned by GetOrganizationLoginPolicy) + // to disregard organization membership for purposes of authentication. + // + // If this list is specified, only teams from the listed organizations should be represented as groups for the downstream token. + GetAllowedOrganizations() []string + + // GetOrganizationLoginPolicy must be "OnlyUsersFromAllowedOrganizations" if GetAllowedOrganizations has values. + // Otherwise, it must be "AllGitHubUsers", which means disregard the result of GetAllowedOrganizations. + GetOrganizationLoginPolicy() v1alpha1.GitHubAllowedAuthOrganizationsPolicy + + // GetAuthorizationURL returns the authorization URL for the configured GitHub. This will look like: + // https:///login/oauth/authorize + // It will not include any query parameters or fragment. Any subdomains or port will come from . + // It will never include a username or password in the authority section. + GetAuthorizationURL() string + + // GetHttpClient returns a http client configured with the provided CA bundle and a timeout. + GetHttpClient() *http.Client } diff --git a/internal/supervisor/server/server.go b/internal/supervisor/server/server.go index 724aba760..2a7d3d739 100644 --- a/internal/supervisor/server/server.go +++ b/internal/supervisor/server/server.go @@ -68,6 +68,7 @@ import ( "go.pinniped.dev/internal/groupsuffix" "go.pinniped.dev/internal/kubeclient" "go.pinniped.dev/internal/leaderelection" + "go.pinniped.dev/internal/net/phttp" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/pversion" "go.pinniped.dev/internal/secret" @@ -325,12 +326,15 @@ func prepareControllers( singletonWorker). WithController( githubupstreamwatcher.New( + podInfo.Namespace, dynamicUpstreamIDPProvider, pinnipedClient, pinnipedInformers.IDP().V1alpha1().GitHubIdentityProviders(), secretInformer, plog.New(), controllerlib.WithInformer, + clock.RealClock{}, + phttp.Default, ), singletonWorker). WithController( diff --git a/internal/testutil/tlsserver/tlsserver.go b/internal/testutil/tlsserver/tlsserver.go index 9669764d4..2fafc78e6 100644 --- a/internal/testutil/tlsserver/tlsserver.go +++ b/internal/testutil/tlsserver/tlsserver.go @@ -30,10 +30,30 @@ const ( helloKey ) +func TLSTestIPv6Server(t *testing.T, handler http.Handler, f func(*httptest.Server)) *httptest.Server { + t.Helper() + + listener, err := net.Listen("tcp6", "[::1]:0") + require.NoError(t, err, "TLSTestIPv6Server: failed to listen on a port") + + server := &httptest.Server{ + Listener: listener, + Config: &http.Server{Handler: handler}, //nolint:gosec //ReadHeaderTimeout is not needed for a localhost listener + } + + return tlsTestServer(t, server, f) +} + func TLSTestServer(t *testing.T, handler http.Handler, f func(*httptest.Server)) *httptest.Server { t.Helper() server := httptest.NewUnstartedServer(handler) + return tlsTestServer(t, server, f) +} + +func tlsTestServer(t *testing.T, server *httptest.Server, f func(*httptest.Server)) *httptest.Server { + t.Helper() + server.TLS = ptls.Default(nil) // mimic API server config if f != nil { f(server) diff --git a/internal/upstreamgithub/upstreamgithub.go b/internal/upstreamgithub/upstreamgithub.go index 4bcddf568..fa9bf1f39 100644 --- a/internal/upstreamgithub/upstreamgithub.go +++ b/internal/upstreamgithub/upstreamgithub.go @@ -1,21 +1,31 @@ // Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -// Package upstreamoidc implements an abstraction of upstream GitHub provider interactions. +// Package upstreamgithub implements an abstraction of upstream GitHub provider interactions. package upstreamgithub import ( + "net/http" + + "golang.org/x/oauth2" "k8s.io/apimachinery/pkg/types" + "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" "go.pinniped.dev/internal/federationdomain/upstreamprovider" ) // ProviderConfig holds the active configuration of an upstream GitHub provider. type ProviderConfig struct { - Name string - ResourceUID types.UID - UsernameClaim string - GroupsClaim string + Name string + ResourceUID types.UID + Host string + UsernameAttribute v1alpha1.GitHubUsernameAttribute + GroupNameAttribute v1alpha1.GitHubGroupNameAttribute + OAuth2Config *oauth2.Config + AllowedOrganizations []string + OrganizationLoginPolicy v1alpha1.GitHubAllowedAuthOrganizationsPolicy + AuthorizationURL string + HttpClient *http.Client } var _ upstreamprovider.UpstreamGithubIdentityProviderI = (*ProviderConfig)(nil) @@ -27,3 +37,35 @@ func (p *ProviderConfig) GetResourceUID() types.UID { func (p *ProviderConfig) GetName() string { return p.Name } + +func (p *ProviderConfig) GetClientID() string { + return p.OAuth2Config.ClientID +} + +func (p *ProviderConfig) GetHost() string { + return p.Host +} + +func (p *ProviderConfig) GetUsernameAttribute() v1alpha1.GitHubUsernameAttribute { + return p.UsernameAttribute +} + +func (p *ProviderConfig) GetGroupNameAttribute() v1alpha1.GitHubGroupNameAttribute { + return p.GroupNameAttribute +} + +func (p *ProviderConfig) GetAllowedOrganizations() []string { + return p.AllowedOrganizations +} + +func (p *ProviderConfig) GetOrganizationLoginPolicy() v1alpha1.GitHubAllowedAuthOrganizationsPolicy { + return p.OrganizationLoginPolicy +} + +func (p *ProviderConfig) GetAuthorizationURL() string { + return p.AuthorizationURL +} + +func (p *ProviderConfig) GetHttpClient() *http.Client { + return p.HttpClient +} diff --git a/test/integration/supervisor_github_idp_test.go b/test/integration/supervisor_github_idp_test.go index 3326bf695..608ae66c2 100644 --- a/test/integration/supervisor_github_idp_test.go +++ b/test/integration/supervisor_github_idp_test.go @@ -21,6 +21,8 @@ import ( "go.pinniped.dev/test/testlib" ) +const generateNamePrefix = "integration-test-github-idp-" + func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { adminClient := testlib.NewKubernetesClientset(t) @@ -32,7 +34,7 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { ns, err := namespaceClient.Create(ctx, &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-github-idp-", + GenerateName: generateNamePrefix, }, }, metav1.CreateOptions{}) require.NoError(t, err) @@ -45,10 +47,10 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { tests := []struct { name string - inputSpec idpv1alpha1.GitHubIdentityProviderSpec - expectedSpec idpv1alpha1.GitHubIdentityProviderSpec usesCELValidation bool - expectedErr string + inputSpec idpv1alpha1.GitHubIdentityProviderSpec + wantSpec idpv1alpha1.GitHubIdentityProviderSpec + wantErr string }{ { name: "all fields set", @@ -78,7 +80,7 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { SecretName: "any-name-goes-here", }, }, - expectedSpec: idpv1alpha1.GitHubIdentityProviderSpec{ + wantSpec: idpv1alpha1.GitHubIdentityProviderSpec{ GitHubAPI: idpv1alpha1.GitHubAPIConfig{ Host: ptr.To("some-host.example.com"), TLS: &idpv1alpha1.TLSSpec{ @@ -115,7 +117,7 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { SecretName: "name-of-a-secret", }, }, - expectedSpec: idpv1alpha1.GitHubIdentityProviderSpec{ + wantSpec: idpv1alpha1.GitHubIdentityProviderSpec{ GitHubAPI: idpv1alpha1.GitHubAPIConfig{ Host: ptr.To("github.com"), }, @@ -137,6 +139,7 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { name: fmt.Sprintf( "cannot set AllowedOrganizationsPolicy=%s and set AllowedOrganizations", string(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers)), + usesCELValidation: true, inputSpec: idpv1alpha1.GitHubIdentityProviderSpec{ AllowAuthentication: idpv1alpha1.GitHubAllowAuthenticationSpec{ Organizations: idpv1alpha1.GitHubOrganizationsSpec{ @@ -150,11 +153,11 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { SecretName: "name-of-a-secret", }, }, - usesCELValidation: true, - expectedErr: "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed", + wantErr: "spec.allowAuthentication.organizations.policy must be 'OnlyUsersFromAllowedOrganizations' when spec.allowAuthentication.organizations.allowed has organizations listed", }, { - name: fmt.Sprintf("spec.allowAuthentication.organizations.policy must be '%s' when spec.allowAuthentication.organizations.allowed is empty (nil)", string(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers)), + name: fmt.Sprintf("spec.allowAuthentication.organizations.policy must be '%s' when spec.allowAuthentication.organizations.allowed is empty (nil)", string(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers)), + usesCELValidation: true, inputSpec: idpv1alpha1.GitHubIdentityProviderSpec{ AllowAuthentication: idpv1alpha1.GitHubAllowAuthenticationSpec{ Organizations: idpv1alpha1.GitHubOrganizationsSpec{ @@ -165,11 +168,11 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { SecretName: "name-of-a-secret", }, }, - usesCELValidation: true, - expectedErr: "spec.allowAuthentication.organizations.policy must be 'AllGitHubUsers' when spec.allowAuthentication.organizations.allowed is empty", + wantErr: "spec.allowAuthentication.organizations.policy must be 'AllGitHubUsers' when spec.allowAuthentication.organizations.allowed is empty", }, { - name: fmt.Sprintf("spec.allowAuthentication.organizations.policy must be '%s' when spec.allowAuthentication.organizations.allowed is empty", string(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers)), + name: fmt.Sprintf("spec.allowAuthentication.organizations.policy must be '%s' when spec.allowAuthentication.organizations.allowed is empty", string(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers)), + usesCELValidation: true, inputSpec: idpv1alpha1.GitHubIdentityProviderSpec{ AllowAuthentication: idpv1alpha1.GitHubAllowAuthenticationSpec{ Organizations: idpv1alpha1.GitHubOrganizationsSpec{ @@ -181,13 +184,12 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { SecretName: "name-of-a-secret", }, }, - usesCELValidation: true, - expectedErr: "spec.allowAuthentication.organizations.policy must be 'AllGitHubUsers' when spec.allowAuthentication.organizations.allowed is empty", + wantErr: "spec.allowAuthentication.organizations.policy must be 'AllGitHubUsers' when spec.allowAuthentication.organizations.allowed is empty", }, { - name: "spec.client.secretName in body should be at least 1 chars long", - inputSpec: idpv1alpha1.GitHubIdentityProviderSpec{}, - expectedErr: "spec.client.secretName in body should be at least 1 chars long", + name: "spec.client.secretName in body should be at least 1 chars long", + inputSpec: idpv1alpha1.GitHubIdentityProviderSpec{}, + wantErr: "spec.client.secretName in body should be at least 1 chars long", }, { name: "spec.githubAPI.host in body should be at least 1 chars long", @@ -204,7 +206,7 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { SecretName: "name-of-a-secret", }, }, - expectedErr: "spec.githubAPI.host in body should be at least 1 chars long", + wantErr: "spec.githubAPI.host in body should be at least 1 chars long", }, { name: "duplicates not permitted in spec.allowAuthentication.organizations.allowed", @@ -222,7 +224,7 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { SecretName: "name-of-a-secret", }, }, - expectedErr: `spec.allowAuthentication.organizations.allowed[1]: Duplicate value: "org1"`, + wantErr: `spec.allowAuthentication.organizations.allowed[1]: Duplicate value: "org1"`, }, } for _, tt := range tests { @@ -236,22 +238,370 @@ func TestGitHubIDPStaticValidationOnCreate_Parallel(t *testing.T) { input := &idpv1alpha1.GitHubIdentityProvider{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "integration-test-", + GenerateName: generateNamePrefix, }, Spec: tt.inputSpec, } outputGitHubIDP, err := gitHubIDPClient.Create(ctx, input, metav1.CreateOptions{}) - if tt.expectedErr == "" { + if tt.wantErr == "" { require.NoError(t, err) - require.Equal(t, tt.expectedSpec, outputGitHubIDP.Spec) + require.Equal(t, tt.wantSpec, outputGitHubIDP.Spec) } else { - require.ErrorContains(t, err, tt.expectedErr) + require.ErrorContains(t, err, tt.wantErr) } }) } } +func TestGitHubIDPPhaseAndConditions_Parallel(t *testing.T) { + // These operations must be performed in the Supervisor's namespace so that the controller can find GitHubIdentityProvider + supervisorNamespace := testlib.IntegrationEnv(t).SupervisorNamespace + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + t.Cleanup(cancel) + + kubernetesClient := testlib.NewKubernetesClientset(t) + secretsClient := kubernetesClient.CoreV1().Secrets(supervisorNamespace) + gitHubIDPClient := testlib.NewSupervisorClientset(t).IDPV1alpha1().GitHubIdentityProviders(supervisorNamespace) + + happySecretName := generateNamePrefix + testlib.RandHex(t, 16) + invalidSecretName := generateNamePrefix + testlib.RandHex(t, 16) + + tests := []struct { + name string + secrets []*corev1.Secret // Secrets will be created first, and the first secret found will be listed as the configured GitHub Client secret + idps []*idpv1alpha1.GitHubIdentityProvider + wantPhase idpv1alpha1.GitHubIdentityProviderPhase + wantConditions []*metav1.Condition + }{ + { + name: "Happy Path", + secrets: []*corev1.Secret{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: happySecretName, + }, + Type: "secrets.pinniped.dev/github-client", + Data: map[string][]byte{ + "clientID": []byte("foo"), + "clientSecret": []byte("bar"), + }, + }, + }, + idps: []*idpv1alpha1.GitHubIdentityProvider{ + { + Spec: idpv1alpha1.GitHubIdentityProviderSpec{ + GitHubAPI: idpv1alpha1.GitHubAPIConfig{ + Host: ptr.To("github.com"), + }, + AllowAuthentication: idpv1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: idpv1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers), + }, + }, + }, + }, + }, + wantPhase: idpv1alpha1.GitHubPhaseReady, + wantConditions: []*metav1.Condition{ + { + Type: "ClientCredentialsObtained", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: fmt.Sprintf("clientID and clientSecret have been read from spec.client.SecretName (%q)", happySecretName), + }, + { + Type: "GitHubConnectionValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: `spec.githubAPI.host ("github.com:443") is reachable and TLS verification succeeds`, + }, + { + Type: "HostValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: `spec.githubAPI.host ("github.com") is valid`, + }, + { + Type: "OrganizationsPolicyValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: `spec.allowAuthentication.organizations.policy ("AllGitHubUsers") is valid`, + }, + { + Type: "TLSConfigurationValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: "spec.githubAPI.tls.certificateAuthorityData is valid", + }, + }, + }, + { + name: "Invalid Client Secret", + secrets: []*corev1.Secret{ + { + Type: "secrets.pinniped.dev/github-client", + ObjectMeta: metav1.ObjectMeta{ + Name: invalidSecretName, + }, + }, + }, + idps: []*idpv1alpha1.GitHubIdentityProvider{ + { + Spec: idpv1alpha1.GitHubIdentityProviderSpec{ + GitHubAPI: idpv1alpha1.GitHubAPIConfig{ + Host: ptr.To("github.com"), + }, + AllowAuthentication: idpv1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: idpv1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers), + }, + }, + Client: idpv1alpha1.GitHubClientSpec{ + SecretName: invalidSecretName, + }, + }, + }, + }, + wantPhase: idpv1alpha1.GitHubPhaseError, + wantConditions: []*metav1.Condition{ + { + Type: "ClientCredentialsObtained", + Status: metav1.ConditionFalse, + Reason: "SecretNotFound", + Message: fmt.Sprintf(`missing key "clientID": secret from spec.client.SecretName (%q) must be found in namespace %q with type "secrets.pinniped.dev/github-client" and keys "clientID" and "clientSecret"`, + invalidSecretName, + supervisorNamespace), + }, + { + Type: "GitHubConnectionValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: `spec.githubAPI.host ("github.com:443") is reachable and TLS verification succeeds`, + }, + { + Type: "HostValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: `spec.githubAPI.host ("github.com") is valid`, + }, + { + Type: "OrganizationsPolicyValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: `spec.allowAuthentication.organizations.policy ("AllGitHubUsers") is valid`, + }, + { + Type: "TLSConfigurationValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: `spec.githubAPI.tls.certificateAuthorityData is valid`, + }, + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var secretName string + for _, secret := range tt.secrets { + secret.GenerateName = generateNamePrefix + + created, err := secretsClient.Create(ctx, secret, metav1.CreateOptions{}) + require.NoError(t, err) + t.Cleanup(func() { + err := secretsClient.Delete(ctx, created.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + if secretName == "" { + secretName = created.Name + } + } + + for _, idp := range tt.idps { + idp.Name = "" + idp.GenerateName = generateNamePrefix + idp.Spec.Client.SecretName = secretName + + created, err := gitHubIDPClient.Create(ctx, idp, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Cleanup(func() { + err := gitHubIDPClient.Delete(ctx, created.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + testlib.WaitForGitHubIDPPhase(ctx, t, gitHubIDPClient, created.Name, tt.wantPhase) + testlib.WaitForGitHubIdentityProviderStatusConditions(ctx, t, gitHubIDPClient, created.Name, tt.wantConditions) + } + }) + } +} + +func TestGitHubIDPInWrongNamespace_Parallel(t *testing.T) { + // The GitHubIdentityProvider must be in the same namespace as the controller + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + t.Cleanup(cancel) + + kubernetesClient := testlib.NewKubernetesClientset(t) + + namespaceClient := kubernetesClient.CoreV1().Namespaces() + otherNamespace, err := namespaceClient.Create(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: generateNamePrefix, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Cleanup(func() { + require.NoError(t, namespaceClient.Delete(ctx, otherNamespace.Name, metav1.DeleteOptions{})) + }) + + gitHubIDPClient := testlib.NewSupervisorClientset(t).IDPV1alpha1().GitHubIdentityProviders(otherNamespace.Name) + + idp := &idpv1alpha1.GitHubIdentityProvider{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: generateNamePrefix, + Namespace: otherNamespace.Name, + }, + Spec: idpv1alpha1.GitHubIdentityProviderSpec{ + GitHubAPI: idpv1alpha1.GitHubAPIConfig{ + Host: ptr.To("github.com"), + }, + AllowAuthentication: idpv1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: idpv1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers), + }, + }, + Client: idpv1alpha1.GitHubClientSpec{ + SecretName: "does-not-matter", + }, + }, + } + + createdIDP, err := gitHubIDPClient.Create(ctx, idp, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Cleanup(func() { + err := gitHubIDPClient.Delete(ctx, createdIDP.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + + // We require that there's never an error + // ... and that the status phase is never anything but Pending + // ... and that there are no status conditions + require.Never(t, func() bool { + idp, err := gitHubIDPClient.Get(ctx, createdIDP.Name, metav1.GetOptions{}) + return err != nil && idp.Status.Phase != idpv1alpha1.GitHubPhasePending && len(idp.Status.Conditions) > 0 + }, 2*time.Minute, 10*time.Second) +} + +func TestGitHubIDPSecretInOtherNamespace_Parallel(t *testing.T) { + // The GitHubIdentityProvider must be in the same namespace as the controller + supervisorNamespace := testlib.IntegrationEnv(t).SupervisorNamespace + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + t.Cleanup(cancel) + + kubernetesClient := testlib.NewKubernetesClientset(t) + gitHubIDPClient := testlib.NewSupervisorClientset(t).IDPV1alpha1().GitHubIdentityProviders(supervisorNamespace) + + namespaceClient := kubernetesClient.CoreV1().Namespaces() + otherNamespace, err := namespaceClient.Create(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: generateNamePrefix, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Cleanup(func() { + require.NoError(t, namespaceClient.Delete(ctx, otherNamespace.Name, metav1.DeleteOptions{})) + }) + + secretsClient := kubernetesClient.CoreV1().Secrets(otherNamespace.Name) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: generateNamePrefix, + Namespace: otherNamespace.Name, + }, + Type: "secrets.pinniped.dev/github-client", + Data: map[string][]byte{ + "clientID": []byte("foo"), + "clientSecret": []byte("bar"), + }, + } + + // This secret will be cleaned up when its namespace is deleted + createdSecret, err := secretsClient.Create(ctx, secret, metav1.CreateOptions{}) + require.NoError(t, err) + + idp := &idpv1alpha1.GitHubIdentityProvider{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: generateNamePrefix, + Namespace: supervisorNamespace, + }, + Spec: idpv1alpha1.GitHubIdentityProviderSpec{ + GitHubAPI: idpv1alpha1.GitHubAPIConfig{ + Host: ptr.To("github.com"), + }, + AllowAuthentication: idpv1alpha1.GitHubAllowAuthenticationSpec{ + Organizations: idpv1alpha1.GitHubOrganizationsSpec{ + Policy: ptr.To(idpv1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers), + }, + }, + Client: idpv1alpha1.GitHubClientSpec{ + SecretName: createdSecret.Name, + }, + }, + } + + created, err := gitHubIDPClient.Create(ctx, idp, metav1.CreateOptions{}) + require.NoError(t, err) + + t.Cleanup(func() { + err := gitHubIDPClient.Delete(ctx, created.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + testlib.WaitForGitHubIDPPhase(ctx, t, gitHubIDPClient, created.Name, idpv1alpha1.GitHubPhaseError) + + testlib.WaitForGitHubIdentityProviderStatusConditions(ctx, t, gitHubIDPClient, created.Name, []*metav1.Condition{ + { + Type: "ClientCredentialsObtained", + Status: metav1.ConditionFalse, + Reason: "SecretNotFound", + Message: fmt.Sprintf(`secret %q not found: secret from spec.client.SecretName (%q) must be found in namespace %q with type "secrets.pinniped.dev/github-client" and keys "clientID" and "clientSecret"`, + idp.Spec.Client.SecretName, + idp.Spec.Client.SecretName, + supervisorNamespace), + }, + { + Type: "GitHubConnectionValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: `spec.githubAPI.host ("github.com:443") is reachable and TLS verification succeeds`, + }, + { + Type: "HostValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: `spec.githubAPI.host ("github.com") is valid`, + }, + { + Type: "OrganizationsPolicyValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: `spec.allowAuthentication.organizations.policy ("AllGitHubUsers") is valid`, + }, + { + Type: "TLSConfigurationValid", + Status: metav1.ConditionTrue, + Reason: "Success", + Message: "spec.githubAPI.tls.certificateAuthorityData is valid", + }, + }) +} + func TestGitHubIDPTooManyOrganizationsStaticValidationOnCreate_Parallel(t *testing.T) { adminClient := testlib.NewKubernetesClientset(t) @@ -262,7 +612,7 @@ func TestGitHubIDPTooManyOrganizationsStaticValidationOnCreate_Parallel(t *testi ns, err := namespaceClient.Create(ctx, &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-github-idp-", + GenerateName: generateNamePrefix, }, }, metav1.CreateOptions{}) require.NoError(t, err) @@ -275,7 +625,7 @@ func TestGitHubIDPTooManyOrganizationsStaticValidationOnCreate_Parallel(t *testi input := &idpv1alpha1.GitHubIdentityProvider{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "integration-test-", + GenerateName: generateNamePrefix, }, Spec: idpv1alpha1.GitHubIdentityProviderSpec{ AllowAuthentication: idpv1alpha1.GitHubAllowAuthenticationSpec{ @@ -298,10 +648,10 @@ func TestGitHubIDPTooManyOrganizationsStaticValidationOnCreate_Parallel(t *testi _, err = gitHubIDPClient.Create(ctx, input, metav1.CreateOptions{}) - expectedErr := "spec.allowAuthentication.organizations.allowed: Invalid value: 100: spec.allowAuthentication.organizations.allowed in body should have at most 64 items" + wantErr := "spec.allowAuthentication.organizations.allowed: Invalid value: 100: spec.allowAuthentication.organizations.allowed in body should have at most 64 items" if testutil.KubeServerMinorVersionAtLeastInclusive(t, adminClient.Discovery(), 24) { - expectedErr = "spec.allowAuthentication.organizations.allowed: Too many: 100: must have at most 64 items" + wantErr = "spec.allowAuthentication.organizations.allowed: Too many: 100: must have at most 64 items" } - require.ErrorContains(t, err, expectedErr) + require.ErrorContains(t, err, wantErr) } diff --git a/test/testlib/client.go b/test/testlib/client.go index 9be61147a..feda851b9 100644 --- a/test/testlib/client.go +++ b/test/testlib/client.go @@ -34,6 +34,7 @@ import ( idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" conciergeclientset "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned" supervisorclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned" + alpha1 "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/typed/idp/v1alpha1" "go.pinniped.dev/internal/groupsuffix" "go.pinniped.dev/internal/kubeclient" @@ -836,6 +837,59 @@ func WaitForUserToHaveAccess(t *testing.T, user string, groups []string, shouldH }, time.Minute, 500*time.Millisecond) } +func WaitForGitHubIDPPhase( + ctx context.Context, + t *testing.T, + client alpha1.GitHubIdentityProviderInterface, + gitHubIDPName string, + expectPhase idpv1alpha1.GitHubIdentityProviderPhase, +) { + t.Helper() + + RequireEventuallyf(t, func(requireEventually *require.Assertions) { + idp, err := client.Get(ctx, gitHubIDPName, metav1.GetOptions{}) + requireEventually.NoError(err) + requireEventually.Equalf(expectPhase, idp.Status.Phase, "actual status conditions were: %#v", idp.Status.Conditions) + }, 60*time.Second, 1*time.Second, "expected the GitHubIDP to have status %q", expectPhase) +} + +func WaitForGitHubIdentityProviderStatusConditions( + ctx context.Context, + t *testing.T, + client alpha1.GitHubIdentityProviderInterface, + gitHubIDPName string, + expectConditions []*metav1.Condition, +) { + t.Helper() + + RequireEventuallyf(t, func(requireEventually *require.Assertions) { + idp, err := client.Get(ctx, gitHubIDPName, metav1.GetOptions{}) + requireEventually.NoError(err) + + actualConditions := make([]*metav1.Condition, len(idp.Status.Conditions)) + for i, c := range idp.Status.Conditions { + actualConditions[i] = c.DeepCopy() + } + + requireEventually.Lenf(actualConditions, len(expectConditions), + "wanted status conditions: %#v", expectConditions) + + for i, wantCond := range expectConditions { + actualCond := actualConditions[i] + + // This is a cheat to avoid needing to make equality assertions on these fields. + requireEventually.NotZero(actualCond.LastTransitionTime) + wantCond.LastTransitionTime = actualCond.LastTransitionTime + requireEventually.NotZero(actualCond.ObservedGeneration) + wantCond.ObservedGeneration = actualCond.ObservedGeneration + + requireEventually.Equalf(wantCond, actualCond, + "wanted status conditions: %#v\nactual status conditions were: %#v\nnot equal at index %d", + expectConditions, &actualConditions, i) + } + }, 60*time.Second, 1*time.Second, "wanted conditions for GitHubIdentityProvider %q", gitHubIDPName) +} + func testObjectMeta(t *testing.T, baseName string) metav1.ObjectMeta { return metav1.ObjectMeta{ GenerateName: fmt.Sprintf("test-%s-", baseName),