Add Conditions to LDAPIdentityProvider's Status and start to fill them

- The ldap_upstream_watcher.go controller validates the bind secret and
  uses the Conditions to report errors. Shares some condition reporting
  logic with its sibling controller oidc_upstream_watcher.go, to the
  extent which is convenient without generics in golang.
This commit is contained in:
Ryan Richard 2021-04-12 13:53:21 -07:00
parent 05571abb74
commit 25c1f0d523
24 changed files with 732 additions and 82 deletions

View File

@ -26,6 +26,13 @@ type LDAPIdentityProviderStatus struct {
// +kubebuilder:default=Pending // +kubebuilder:default=Pending
// +kubebuilder:validation:Enum=Pending;Ready;Error // +kubebuilder:validation:Enum=Pending;Ready;Error
Phase LDAPIdentityProviderPhase `json:"phase,omitempty"` Phase LDAPIdentityProviderPhase `json:"phase,omitempty"`
// Represents the observations of an identity provider's current state.
// +patchMergeKey=type
// +patchStrategy=merge
// +listType=map
// +listMapKey=type
Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
} }
type LDAPIdentityProviderTLSSpec struct { type LDAPIdentityProviderTLSSpec struct {

View File

@ -131,6 +131,73 @@ spec:
status: status:
description: Status of the identity provider. description: Status of the identity provider.
properties: properties:
conditions:
description: Represents the observations of an identity provider's
current state.
items:
description: Condition status of a resource (mirrored from the metav1.Condition
type added in Kubernetes 1.19). In a future API version we can
switch to using the upstream type. See https://github.com/kubernetes/apimachinery/blob/v0.19.0/pkg/apis/meta/v1/types.go#L1353-L1413.
properties:
lastTransitionTime:
description: lastTransitionTime is the last time the condition
transitioned from one status to another. This should be when
the underlying condition changed. If that is not known, then
using the time when the API field changed is acceptable.
format: date-time
type: string
message:
description: message is a human readable message indicating
details about the transition. This may be an empty string.
maxLength: 32768
type: string
observedGeneration:
description: observedGeneration represents the .metadata.generation
that the condition was set based upon. For instance, if .metadata.generation
is currently 12, but the .status.conditions[x].observedGeneration
is 9, the condition is out of date with respect to the current
state of the instance.
format: int64
minimum: 0
type: integer
reason:
description: reason contains a programmatic identifier indicating
the reason for the condition's last transition. Producers
of specific condition types may define expected values and
meanings for this field, and whether the values are considered
a guaranteed API. The value should be a CamelCase string.
This field may not be empty.
maxLength: 1024
minLength: 1
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
type: string
status:
description: status of the condition, one of True, False, Unknown.
enum:
- "True"
- "False"
- Unknown
type: string
type:
description: type of condition in CamelCase or in foo.example.com/CamelCase.
--- Many .condition.type values are consistent across resources
like Available, but because arbitrary conditions can be useful
(see .node.status.conditions), the ability to deconflict is
important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
maxLength: 316
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
type: string
required:
- lastTransitionTime
- message
- reason
- status
- type
type: object
type: array
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
phase: phase:
default: Pending default: Pending
description: Phase summarizes the overall status of the LDAPIdentityProvider. description: Phase summarizes the overall status of the LDAPIdentityProvider.

View File

@ -669,10 +669,11 @@ Package v1alpha1 is the v1alpha1 version of the Pinniped supervisor identity pro
[id="{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-condition"] [id="{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-condition"]
==== Condition ==== Condition
Condition status of a resource (mirrored from the metav1.Condition type added in Kubernetes 1.19). In a future API version we can switch to using the upstream type. See https://github.com/kubernetes/apimachinery/blob/v0.19.0/pkg/apis/meta/v1/types.go#L1353-L1413.
.Appears In: .Appears In:
**** ****
- xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-ldapidentityproviderstatus[$$LDAPIdentityProviderStatus$$]
- xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-oidcidentityproviderstatus[$$OIDCIdentityProviderStatus$$] - xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-oidcidentityproviderstatus[$$OIDCIdentityProviderStatus$$]
**** ****
@ -688,6 +689,18 @@ Condition status of a resource (mirrored from the metav1.Condition type added in
|=== |===
[id="{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-conditionstatus"]
==== ConditionStatus (string)
.Appears In:
****
- xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-condition[$$Condition$$]
****
[id="{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-ldapidentityprovider"] [id="{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-ldapidentityprovider"]
==== LDAPIdentityProvider ==== LDAPIdentityProvider
@ -761,6 +774,7 @@ Status of an LDAP identity provider.
|=== |===
| Field | Description | Field | Description
| *`phase`* __LDAPIdentityProviderPhase__ | Phase summarizes the overall status of the LDAPIdentityProvider. | *`phase`* __LDAPIdentityProviderPhase__ | Phase summarizes the overall status of the LDAPIdentityProvider.
| *`conditions`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-condition[$$Condition$$] array__ | Represents the observations of an identity provider's current state.
|=== |===
@ -927,7 +941,7 @@ Status of an OIDC identity provider.
|=== |===
| Field | Description | Field | Description
| *`phase`* __OIDCIdentityProviderPhase__ | Phase summarizes the overall status of the OIDCIdentityProvider. | *`phase`* __OIDCIdentityProviderPhase__ | Phase summarizes the overall status of the OIDCIdentityProvider.
| *`conditions`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-condition[$$Condition$$]__ | Represents the observations of an identity provider's current state. | *`conditions`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-17-apis-supervisor-idp-v1alpha1-condition[$$Condition$$] array__ | Represents the observations of an identity provider's current state.
|=== |===

View File

@ -26,6 +26,13 @@ type LDAPIdentityProviderStatus struct {
// +kubebuilder:default=Pending // +kubebuilder:default=Pending
// +kubebuilder:validation:Enum=Pending;Ready;Error // +kubebuilder:validation:Enum=Pending;Ready;Error
Phase LDAPIdentityProviderPhase `json:"phase,omitempty"` Phase LDAPIdentityProviderPhase `json:"phase,omitempty"`
// Represents the observations of an identity provider's current state.
// +patchMergeKey=type
// +patchStrategy=merge
// +listType=map
// +listMapKey=type
Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
} }
type LDAPIdentityProviderTLSSpec struct { type LDAPIdentityProviderTLSSpec struct {

View File

@ -34,7 +34,7 @@ func (in *LDAPIdentityProvider) DeepCopyInto(out *LDAPIdentityProvider) {
out.TypeMeta = in.TypeMeta out.TypeMeta = in.TypeMeta
in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
in.Spec.DeepCopyInto(&out.Spec) in.Spec.DeepCopyInto(&out.Spec)
out.Status = in.Status in.Status.DeepCopyInto(&out.Status)
return return
} }
@ -131,6 +131,13 @@ func (in *LDAPIdentityProviderSpec) DeepCopy() *LDAPIdentityProviderSpec {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *LDAPIdentityProviderStatus) DeepCopyInto(out *LDAPIdentityProviderStatus) { func (in *LDAPIdentityProviderStatus) DeepCopyInto(out *LDAPIdentityProviderStatus) {
*out = *in *out = *in
if in.Conditions != nil {
in, out := &in.Conditions, &out.Conditions
*out = make([]Condition, len(*in))
for i := range *in {
(*in)[i].DeepCopyInto(&(*out)[i])
}
}
return return
} }

View File

@ -131,6 +131,73 @@ spec:
status: status:
description: Status of the identity provider. description: Status of the identity provider.
properties: properties:
conditions:
description: Represents the observations of an identity provider's
current state.
items:
description: Condition status of a resource (mirrored from the metav1.Condition
type added in Kubernetes 1.19). In a future API version we can
switch to using the upstream type. See https://github.com/kubernetes/apimachinery/blob/v0.19.0/pkg/apis/meta/v1/types.go#L1353-L1413.
properties:
lastTransitionTime:
description: lastTransitionTime is the last time the condition
transitioned from one status to another. This should be when
the underlying condition changed. If that is not known, then
using the time when the API field changed is acceptable.
format: date-time
type: string
message:
description: message is a human readable message indicating
details about the transition. This may be an empty string.
maxLength: 32768
type: string
observedGeneration:
description: observedGeneration represents the .metadata.generation
that the condition was set based upon. For instance, if .metadata.generation
is currently 12, but the .status.conditions[x].observedGeneration
is 9, the condition is out of date with respect to the current
state of the instance.
format: int64
minimum: 0
type: integer
reason:
description: reason contains a programmatic identifier indicating
the reason for the condition's last transition. Producers
of specific condition types may define expected values and
meanings for this field, and whether the values are considered
a guaranteed API. The value should be a CamelCase string.
This field may not be empty.
maxLength: 1024
minLength: 1
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
type: string
status:
description: status of the condition, one of True, False, Unknown.
enum:
- "True"
- "False"
- Unknown
type: string
type:
description: type of condition in CamelCase or in foo.example.com/CamelCase.
--- Many .condition.type values are consistent across resources
like Available, but because arbitrary conditions can be useful
(see .node.status.conditions), the ability to deconflict is
important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
maxLength: 316
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
type: string
required:
- lastTransitionTime
- message
- reason
- status
- type
type: object
type: array
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
phase: phase:
default: Pending default: Pending
description: Phase summarizes the overall status of the LDAPIdentityProvider. description: Phase summarizes the overall status of the LDAPIdentityProvider.

View File

@ -669,10 +669,11 @@ Package v1alpha1 is the v1alpha1 version of the Pinniped supervisor identity pro
[id="{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-condition"] [id="{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-condition"]
==== Condition ==== Condition
Condition status of a resource (mirrored from the metav1.Condition type added in Kubernetes 1.19). In a future API version we can switch to using the upstream type. See https://github.com/kubernetes/apimachinery/blob/v0.19.0/pkg/apis/meta/v1/types.go#L1353-L1413.
.Appears In: .Appears In:
**** ****
- xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-ldapidentityproviderstatus[$$LDAPIdentityProviderStatus$$]
- xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-oidcidentityproviderstatus[$$OIDCIdentityProviderStatus$$] - xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-oidcidentityproviderstatus[$$OIDCIdentityProviderStatus$$]
**** ****
@ -688,6 +689,18 @@ Condition status of a resource (mirrored from the metav1.Condition type added in
|=== |===
[id="{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-conditionstatus"]
==== ConditionStatus (string)
.Appears In:
****
- xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-condition[$$Condition$$]
****
[id="{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-ldapidentityprovider"] [id="{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-ldapidentityprovider"]
==== LDAPIdentityProvider ==== LDAPIdentityProvider
@ -761,6 +774,7 @@ Status of an LDAP identity provider.
|=== |===
| Field | Description | Field | Description
| *`phase`* __LDAPIdentityProviderPhase__ | Phase summarizes the overall status of the LDAPIdentityProvider. | *`phase`* __LDAPIdentityProviderPhase__ | Phase summarizes the overall status of the LDAPIdentityProvider.
| *`conditions`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-condition[$$Condition$$] array__ | Represents the observations of an identity provider's current state.
|=== |===
@ -927,7 +941,7 @@ Status of an OIDC identity provider.
|=== |===
| Field | Description | Field | Description
| *`phase`* __OIDCIdentityProviderPhase__ | Phase summarizes the overall status of the OIDCIdentityProvider. | *`phase`* __OIDCIdentityProviderPhase__ | Phase summarizes the overall status of the OIDCIdentityProvider.
| *`conditions`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-condition[$$Condition$$]__ | Represents the observations of an identity provider's current state. | *`conditions`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-18-apis-supervisor-idp-v1alpha1-condition[$$Condition$$] array__ | Represents the observations of an identity provider's current state.
|=== |===

View File

@ -26,6 +26,13 @@ type LDAPIdentityProviderStatus struct {
// +kubebuilder:default=Pending // +kubebuilder:default=Pending
// +kubebuilder:validation:Enum=Pending;Ready;Error // +kubebuilder:validation:Enum=Pending;Ready;Error
Phase LDAPIdentityProviderPhase `json:"phase,omitempty"` Phase LDAPIdentityProviderPhase `json:"phase,omitempty"`
// Represents the observations of an identity provider's current state.
// +patchMergeKey=type
// +patchStrategy=merge
// +listType=map
// +listMapKey=type
Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
} }
type LDAPIdentityProviderTLSSpec struct { type LDAPIdentityProviderTLSSpec struct {

View File

@ -34,7 +34,7 @@ func (in *LDAPIdentityProvider) DeepCopyInto(out *LDAPIdentityProvider) {
out.TypeMeta = in.TypeMeta out.TypeMeta = in.TypeMeta
in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
in.Spec.DeepCopyInto(&out.Spec) in.Spec.DeepCopyInto(&out.Spec)
out.Status = in.Status in.Status.DeepCopyInto(&out.Status)
return return
} }
@ -131,6 +131,13 @@ func (in *LDAPIdentityProviderSpec) DeepCopy() *LDAPIdentityProviderSpec {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *LDAPIdentityProviderStatus) DeepCopyInto(out *LDAPIdentityProviderStatus) { func (in *LDAPIdentityProviderStatus) DeepCopyInto(out *LDAPIdentityProviderStatus) {
*out = *in *out = *in
if in.Conditions != nil {
in, out := &in.Conditions, &out.Conditions
*out = make([]Condition, len(*in))
for i := range *in {
(*in)[i].DeepCopyInto(&(*out)[i])
}
}
return return
} }

View File

@ -131,6 +131,73 @@ spec:
status: status:
description: Status of the identity provider. description: Status of the identity provider.
properties: properties:
conditions:
description: Represents the observations of an identity provider's
current state.
items:
description: Condition status of a resource (mirrored from the metav1.Condition
type added in Kubernetes 1.19). In a future API version we can
switch to using the upstream type. See https://github.com/kubernetes/apimachinery/blob/v0.19.0/pkg/apis/meta/v1/types.go#L1353-L1413.
properties:
lastTransitionTime:
description: lastTransitionTime is the last time the condition
transitioned from one status to another. This should be when
the underlying condition changed. If that is not known, then
using the time when the API field changed is acceptable.
format: date-time
type: string
message:
description: message is a human readable message indicating
details about the transition. This may be an empty string.
maxLength: 32768
type: string
observedGeneration:
description: observedGeneration represents the .metadata.generation
that the condition was set based upon. For instance, if .metadata.generation
is currently 12, but the .status.conditions[x].observedGeneration
is 9, the condition is out of date with respect to the current
state of the instance.
format: int64
minimum: 0
type: integer
reason:
description: reason contains a programmatic identifier indicating
the reason for the condition's last transition. Producers
of specific condition types may define expected values and
meanings for this field, and whether the values are considered
a guaranteed API. The value should be a CamelCase string.
This field may not be empty.
maxLength: 1024
minLength: 1
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
type: string
status:
description: status of the condition, one of True, False, Unknown.
enum:
- "True"
- "False"
- Unknown
type: string
type:
description: type of condition in CamelCase or in foo.example.com/CamelCase.
--- Many .condition.type values are consistent across resources
like Available, but because arbitrary conditions can be useful
(see .node.status.conditions), the ability to deconflict is
important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
maxLength: 316
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
type: string
required:
- lastTransitionTime
- message
- reason
- status
- type
type: object
type: array
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
phase: phase:
default: Pending default: Pending
description: Phase summarizes the overall status of the LDAPIdentityProvider. description: Phase summarizes the overall status of the LDAPIdentityProvider.

View File

@ -669,10 +669,11 @@ Package v1alpha1 is the v1alpha1 version of the Pinniped supervisor identity pro
[id="{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-condition"] [id="{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-condition"]
==== Condition ==== Condition
Condition status of a resource (mirrored from the metav1.Condition type added in Kubernetes 1.19). In a future API version we can switch to using the upstream type. See https://github.com/kubernetes/apimachinery/blob/v0.19.0/pkg/apis/meta/v1/types.go#L1353-L1413.
.Appears In: .Appears In:
**** ****
- xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-ldapidentityproviderstatus[$$LDAPIdentityProviderStatus$$]
- xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-oidcidentityproviderstatus[$$OIDCIdentityProviderStatus$$] - xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-oidcidentityproviderstatus[$$OIDCIdentityProviderStatus$$]
**** ****
@ -688,6 +689,18 @@ Condition status of a resource (mirrored from the metav1.Condition type added in
|=== |===
[id="{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-conditionstatus"]
==== ConditionStatus (string)
.Appears In:
****
- xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-condition[$$Condition$$]
****
[id="{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-ldapidentityprovider"] [id="{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-ldapidentityprovider"]
==== LDAPIdentityProvider ==== LDAPIdentityProvider
@ -761,6 +774,7 @@ Status of an LDAP identity provider.
|=== |===
| Field | Description | Field | Description
| *`phase`* __LDAPIdentityProviderPhase__ | Phase summarizes the overall status of the LDAPIdentityProvider. | *`phase`* __LDAPIdentityProviderPhase__ | Phase summarizes the overall status of the LDAPIdentityProvider.
| *`conditions`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-condition[$$Condition$$] array__ | Represents the observations of an identity provider's current state.
|=== |===
@ -927,7 +941,7 @@ Status of an OIDC identity provider.
|=== |===
| Field | Description | Field | Description
| *`phase`* __OIDCIdentityProviderPhase__ | Phase summarizes the overall status of the OIDCIdentityProvider. | *`phase`* __OIDCIdentityProviderPhase__ | Phase summarizes the overall status of the OIDCIdentityProvider.
| *`conditions`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-condition[$$Condition$$]__ | Represents the observations of an identity provider's current state. | *`conditions`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-19-apis-supervisor-idp-v1alpha1-condition[$$Condition$$] array__ | Represents the observations of an identity provider's current state.
|=== |===

View File

@ -26,6 +26,13 @@ type LDAPIdentityProviderStatus struct {
// +kubebuilder:default=Pending // +kubebuilder:default=Pending
// +kubebuilder:validation:Enum=Pending;Ready;Error // +kubebuilder:validation:Enum=Pending;Ready;Error
Phase LDAPIdentityProviderPhase `json:"phase,omitempty"` Phase LDAPIdentityProviderPhase `json:"phase,omitempty"`
// Represents the observations of an identity provider's current state.
// +patchMergeKey=type
// +patchStrategy=merge
// +listType=map
// +listMapKey=type
Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
} }
type LDAPIdentityProviderTLSSpec struct { type LDAPIdentityProviderTLSSpec struct {

View File

@ -34,7 +34,7 @@ func (in *LDAPIdentityProvider) DeepCopyInto(out *LDAPIdentityProvider) {
out.TypeMeta = in.TypeMeta out.TypeMeta = in.TypeMeta
in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
in.Spec.DeepCopyInto(&out.Spec) in.Spec.DeepCopyInto(&out.Spec)
out.Status = in.Status in.Status.DeepCopyInto(&out.Status)
return return
} }
@ -131,6 +131,13 @@ func (in *LDAPIdentityProviderSpec) DeepCopy() *LDAPIdentityProviderSpec {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *LDAPIdentityProviderStatus) DeepCopyInto(out *LDAPIdentityProviderStatus) { func (in *LDAPIdentityProviderStatus) DeepCopyInto(out *LDAPIdentityProviderStatus) {
*out = *in *out = *in
if in.Conditions != nil {
in, out := &in.Conditions, &out.Conditions
*out = make([]Condition, len(*in))
for i := range *in {
(*in)[i].DeepCopyInto(&(*out)[i])
}
}
return return
} }

View File

@ -131,6 +131,73 @@ spec:
status: status:
description: Status of the identity provider. description: Status of the identity provider.
properties: properties:
conditions:
description: Represents the observations of an identity provider's
current state.
items:
description: Condition status of a resource (mirrored from the metav1.Condition
type added in Kubernetes 1.19). In a future API version we can
switch to using the upstream type. See https://github.com/kubernetes/apimachinery/blob/v0.19.0/pkg/apis/meta/v1/types.go#L1353-L1413.
properties:
lastTransitionTime:
description: lastTransitionTime is the last time the condition
transitioned from one status to another. This should be when
the underlying condition changed. If that is not known, then
using the time when the API field changed is acceptable.
format: date-time
type: string
message:
description: message is a human readable message indicating
details about the transition. This may be an empty string.
maxLength: 32768
type: string
observedGeneration:
description: observedGeneration represents the .metadata.generation
that the condition was set based upon. For instance, if .metadata.generation
is currently 12, but the .status.conditions[x].observedGeneration
is 9, the condition is out of date with respect to the current
state of the instance.
format: int64
minimum: 0
type: integer
reason:
description: reason contains a programmatic identifier indicating
the reason for the condition's last transition. Producers
of specific condition types may define expected values and
meanings for this field, and whether the values are considered
a guaranteed API. The value should be a CamelCase string.
This field may not be empty.
maxLength: 1024
minLength: 1
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
type: string
status:
description: status of the condition, one of True, False, Unknown.
enum:
- "True"
- "False"
- Unknown
type: string
type:
description: type of condition in CamelCase or in foo.example.com/CamelCase.
--- Many .condition.type values are consistent across resources
like Available, but because arbitrary conditions can be useful
(see .node.status.conditions), the ability to deconflict is
important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
maxLength: 316
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
type: string
required:
- lastTransitionTime
- message
- reason
- status
- type
type: object
type: array
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
phase: phase:
default: Pending default: Pending
description: Phase summarizes the overall status of the LDAPIdentityProvider. description: Phase summarizes the overall status of the LDAPIdentityProvider.

View File

@ -669,10 +669,11 @@ Package v1alpha1 is the v1alpha1 version of the Pinniped supervisor identity pro
[id="{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-condition"] [id="{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-condition"]
==== Condition ==== Condition
Condition status of a resource (mirrored from the metav1.Condition type added in Kubernetes 1.19). In a future API version we can switch to using the upstream type. See https://github.com/kubernetes/apimachinery/blob/v0.19.0/pkg/apis/meta/v1/types.go#L1353-L1413.
.Appears In: .Appears In:
**** ****
- xref:{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-ldapidentityproviderstatus[$$LDAPIdentityProviderStatus$$]
- xref:{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-oidcidentityproviderstatus[$$OIDCIdentityProviderStatus$$] - xref:{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-oidcidentityproviderstatus[$$OIDCIdentityProviderStatus$$]
**** ****
@ -688,6 +689,18 @@ Condition status of a resource (mirrored from the metav1.Condition type added in
|=== |===
[id="{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-conditionstatus"]
==== ConditionStatus (string)
.Appears In:
****
- xref:{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-condition[$$Condition$$]
****
[id="{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-ldapidentityprovider"] [id="{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-ldapidentityprovider"]
==== LDAPIdentityProvider ==== LDAPIdentityProvider
@ -761,6 +774,7 @@ Status of an LDAP identity provider.
|=== |===
| Field | Description | Field | Description
| *`phase`* __LDAPIdentityProviderPhase__ | Phase summarizes the overall status of the LDAPIdentityProvider. | *`phase`* __LDAPIdentityProviderPhase__ | Phase summarizes the overall status of the LDAPIdentityProvider.
| *`conditions`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-condition[$$Condition$$] array__ | Represents the observations of an identity provider's current state.
|=== |===
@ -927,7 +941,7 @@ Status of an OIDC identity provider.
|=== |===
| Field | Description | Field | Description
| *`phase`* __OIDCIdentityProviderPhase__ | Phase summarizes the overall status of the OIDCIdentityProvider. | *`phase`* __OIDCIdentityProviderPhase__ | Phase summarizes the overall status of the OIDCIdentityProvider.
| *`conditions`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-condition[$$Condition$$]__ | Represents the observations of an identity provider's current state. | *`conditions`* __xref:{anchor_prefix}-go-pinniped-dev-generated-1-20-apis-supervisor-idp-v1alpha1-condition[$$Condition$$] array__ | Represents the observations of an identity provider's current state.
|=== |===

View File

@ -26,6 +26,13 @@ type LDAPIdentityProviderStatus struct {
// +kubebuilder:default=Pending // +kubebuilder:default=Pending
// +kubebuilder:validation:Enum=Pending;Ready;Error // +kubebuilder:validation:Enum=Pending;Ready;Error
Phase LDAPIdentityProviderPhase `json:"phase,omitempty"` Phase LDAPIdentityProviderPhase `json:"phase,omitempty"`
// Represents the observations of an identity provider's current state.
// +patchMergeKey=type
// +patchStrategy=merge
// +listType=map
// +listMapKey=type
Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
} }
type LDAPIdentityProviderTLSSpec struct { type LDAPIdentityProviderTLSSpec struct {

View File

@ -34,7 +34,7 @@ func (in *LDAPIdentityProvider) DeepCopyInto(out *LDAPIdentityProvider) {
out.TypeMeta = in.TypeMeta out.TypeMeta = in.TypeMeta
in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
in.Spec.DeepCopyInto(&out.Spec) in.Spec.DeepCopyInto(&out.Spec)
out.Status = in.Status in.Status.DeepCopyInto(&out.Status)
return return
} }
@ -131,6 +131,13 @@ func (in *LDAPIdentityProviderSpec) DeepCopy() *LDAPIdentityProviderSpec {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *LDAPIdentityProviderStatus) DeepCopyInto(out *LDAPIdentityProviderStatus) { func (in *LDAPIdentityProviderStatus) DeepCopyInto(out *LDAPIdentityProviderStatus) {
*out = *in *out = *in
if in.Conditions != nil {
in, out := &in.Conditions, &out.Conditions
*out = make([]Condition, len(*in))
for i := range *in {
(*in)[i].DeepCopyInto(&(*out)[i])
}
}
return return
} }

View File

@ -131,6 +131,73 @@ spec:
status: status:
description: Status of the identity provider. description: Status of the identity provider.
properties: properties:
conditions:
description: Represents the observations of an identity provider's
current state.
items:
description: Condition status of a resource (mirrored from the metav1.Condition
type added in Kubernetes 1.19). In a future API version we can
switch to using the upstream type. See https://github.com/kubernetes/apimachinery/blob/v0.19.0/pkg/apis/meta/v1/types.go#L1353-L1413.
properties:
lastTransitionTime:
description: lastTransitionTime is the last time the condition
transitioned from one status to another. This should be when
the underlying condition changed. If that is not known, then
using the time when the API field changed is acceptable.
format: date-time
type: string
message:
description: message is a human readable message indicating
details about the transition. This may be an empty string.
maxLength: 32768
type: string
observedGeneration:
description: observedGeneration represents the .metadata.generation
that the condition was set based upon. For instance, if .metadata.generation
is currently 12, but the .status.conditions[x].observedGeneration
is 9, the condition is out of date with respect to the current
state of the instance.
format: int64
minimum: 0
type: integer
reason:
description: reason contains a programmatic identifier indicating
the reason for the condition's last transition. Producers
of specific condition types may define expected values and
meanings for this field, and whether the values are considered
a guaranteed API. The value should be a CamelCase string.
This field may not be empty.
maxLength: 1024
minLength: 1
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
type: string
status:
description: status of the condition, one of True, False, Unknown.
enum:
- "True"
- "False"
- Unknown
type: string
type:
description: type of condition in CamelCase or in foo.example.com/CamelCase.
--- Many .condition.type values are consistent across resources
like Available, but because arbitrary conditions can be useful
(see .node.status.conditions), the ability to deconflict is
important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
maxLength: 316
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
type: string
required:
- lastTransitionTime
- message
- reason
- status
- type
type: object
type: array
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
phase: phase:
default: Pending default: Pending
description: Phase summarizes the overall status of the LDAPIdentityProvider. description: Phase summarizes the overall status of the LDAPIdentityProvider.

View File

@ -26,6 +26,13 @@ type LDAPIdentityProviderStatus struct {
// +kubebuilder:default=Pending // +kubebuilder:default=Pending
// +kubebuilder:validation:Enum=Pending;Ready;Error // +kubebuilder:validation:Enum=Pending;Ready;Error
Phase LDAPIdentityProviderPhase `json:"phase,omitempty"` Phase LDAPIdentityProviderPhase `json:"phase,omitempty"`
// Represents the observations of an identity provider's current state.
// +patchMergeKey=type
// +patchStrategy=merge
// +listType=map
// +listMapKey=type
Conditions []Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
} }
type LDAPIdentityProviderTLSSpec struct { type LDAPIdentityProviderTLSSpec struct {

View File

@ -34,7 +34,7 @@ func (in *LDAPIdentityProvider) DeepCopyInto(out *LDAPIdentityProvider) {
out.TypeMeta = in.TypeMeta out.TypeMeta = in.TypeMeta
in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
in.Spec.DeepCopyInto(&out.Spec) in.Spec.DeepCopyInto(&out.Spec)
out.Status = in.Status in.Status.DeepCopyInto(&out.Status)
return return
} }
@ -131,6 +131,13 @@ func (in *LDAPIdentityProviderSpec) DeepCopy() *LDAPIdentityProviderSpec {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *LDAPIdentityProviderStatus) DeepCopyInto(out *LDAPIdentityProviderStatus) { func (in *LDAPIdentityProviderStatus) DeepCopyInto(out *LDAPIdentityProviderStatus) {
*out = *in *out = *in
if in.Conditions != nil {
in, out := &in.Conditions, &out.Conditions
*out = make([]Condition, len(*in))
for i := range *in {
(*in)[i].DeepCopyInto(&(*out)[i])
}
}
return return
} }

View File

@ -4,11 +4,15 @@
package upstreamwatcher package upstreamwatcher
import ( import (
"context"
"fmt" "fmt"
corev1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/labels"
corev1informers "k8s.io/client-go/informers/core/v1" corev1informers "k8s.io/client-go/informers/core/v1"
"k8s.io/klog/v2/klogr"
"go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
pinnipedclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned" pinnipedclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned"
@ -22,6 +26,9 @@ import (
const ( const (
ldapControllerName = "ldap-upstream-observer" ldapControllerName = "ldap-upstream-observer"
ldapBindAccountSecretType = corev1.SecretTypeBasicAuth ldapBindAccountSecretType = corev1.SecretTypeBasicAuth
// Constants related to conditions.
typeBindSecretValid = "BindSecretValid"
) )
// UpstreamLDAPIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations. // UpstreamLDAPIdentityProviderICache is a thread safe cache that holds a list of validated upstream LDAP IDP configurations.
@ -78,7 +85,7 @@ func (c *ldapWatcherController) Sync(ctx controllerlib.Context) error {
requeue := false requeue := false
validatedUpstreams := make([]provider.UpstreamLDAPIdentityProviderI, 0, len(actualUpstreams)) validatedUpstreams := make([]provider.UpstreamLDAPIdentityProviderI, 0, len(actualUpstreams))
for _, upstream := range actualUpstreams { for _, upstream := range actualUpstreams {
valid := c.validateUpstream(upstream) valid := c.validateUpstream(ctx.Context, upstream)
if valid == nil { if valid == nil {
requeue = true requeue = true
} else { } else {
@ -92,7 +99,7 @@ func (c *ldapWatcherController) Sync(ctx controllerlib.Context) error {
return nil return nil
} }
func (c *ldapWatcherController) validateUpstream(upstream *v1alpha1.LDAPIdentityProvider) provider.UpstreamLDAPIdentityProviderI { func (c *ldapWatcherController) validateUpstream(ctx context.Context, upstream *v1alpha1.LDAPIdentityProvider) provider.UpstreamLDAPIdentityProviderI {
spec := upstream.Spec spec := upstream.Spec
result := &upstreamldap.Provider{ result := &upstreamldap.Provider{
Name: upstream.Name, Name: upstream.Name,
@ -106,7 +113,13 @@ func (c *ldapWatcherController) validateUpstream(upstream *v1alpha1.LDAPIdentity
}, },
Dialer: c.ldapDialer, Dialer: c.ldapDialer,
} }
_ = c.validateSecret(upstream, result) conditions := []*v1alpha1.Condition{
c.validateSecret(upstream, result),
}
hadErrorCondition := c.updateStatus(ctx, upstream, conditions)
if hadErrorCondition {
return nil
}
return result return result
} }
@ -115,22 +128,64 @@ func (c ldapWatcherController) validateSecret(upstream *v1alpha1.LDAPIdentityPro
secret, err := c.secretInformer.Lister().Secrets(upstream.Namespace).Get(secretName) secret, err := c.secretInformer.Lister().Secrets(upstream.Namespace).Get(secretName)
if err != nil { if err != nil {
// TODO return &v1alpha1.Condition{
return nil Type: typeBindSecretValid,
Status: v1alpha1.ConditionFalse,
Reason: reasonNotFound,
Message: err.Error(),
}
} }
if secret.Type != corev1.SecretTypeBasicAuth { if secret.Type != corev1.SecretTypeBasicAuth {
// TODO return &v1alpha1.Condition{
return nil Type: typeBindSecretValid,
Status: v1alpha1.ConditionFalse,
Reason: reasonWrongType,
Message: fmt.Sprintf("referenced Secret %q has wrong type %q (should be %q)", secretName, secret.Type, corev1.SecretTypeBasicAuth),
}
} }
result.BindUsername = string(secret.Data[corev1.BasicAuthUsernameKey]) result.BindUsername = string(secret.Data[corev1.BasicAuthUsernameKey])
result.BindPassword = string(secret.Data[corev1.BasicAuthPasswordKey]) result.BindPassword = string(secret.Data[corev1.BasicAuthPasswordKey])
if len(result.BindUsername) == 0 || len(result.BindPassword) == 0 { if len(result.BindUsername) == 0 || len(result.BindPassword) == 0 {
// TODO return &v1alpha1.Condition{
return nil Type: typeBindSecretValid,
Status: v1alpha1.ConditionFalse,
Reason: reasonMissingKeys,
Message: fmt.Sprintf("referenced Secret %q is missing required keys %q", secretName, []string{corev1.BasicAuthUsernameKey, corev1.BasicAuthPasswordKey}),
}
} }
var cond *v1alpha1.Condition // satisfy linter return &v1alpha1.Condition{
return cond Type: typeBindSecretValid,
Status: v1alpha1.ConditionTrue,
Reason: reasonSuccess,
Message: "loaded bind secret",
}
}
func (c *ldapWatcherController) updateStatus(ctx context.Context, upstream *v1alpha1.LDAPIdentityProvider, conditions []*v1alpha1.Condition) bool {
log := klogr.New().WithValues("namespace", upstream.Namespace, "name", upstream.Name)
updated := upstream.DeepCopy()
hadErrorCondition := mergeConditions(conditions, upstream.Generation, &updated.Status.Conditions, log)
updated.Status.Phase = v1alpha1.LDAPPhaseReady
if hadErrorCondition {
updated.Status.Phase = v1alpha1.LDAPPhaseError
}
if equality.Semantic.DeepEqual(upstream, updated) {
return hadErrorCondition
}
_, err := c.client.
IDPV1alpha1().
LDAPIdentityProviders(upstream.Namespace).
UpdateStatus(ctx, updated, metav1.UpdateOptions{})
if err != nil {
log.Error(err, "failed to update status")
}
return hadErrorCondition
} }

View File

@ -5,7 +5,9 @@ package upstreamwatcher
import ( import (
"context" "context"
"fmt"
"testing" "testing"
"time"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1"
@ -138,11 +140,12 @@ func (d *comparableDialer) Dial(ctx context.Context, hostAndPort string) (upstre
func TestLDAPUpstreamWatcherControllerSync(t *testing.T) { func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
t.Parallel() t.Parallel()
now := metav1.NewTime(time.Now().UTC())
const ( const (
testNamespace = "test-namespace" testNamespace = "test-namespace"
testName = "test-name" testName = "test-name"
testSecretName = "test-client-secret" testSecretName = "test-bind-secret"
testBindUsername = "test-bind-username" testBindUsername = "test-bind-username"
testBindPassword = "test-bind-password" testBindPassword = "test-bind-password"
testHost = "ldap.example.com:123" testHost = "ldap.example.com:123"
@ -163,6 +166,38 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
}, },
} }
validUpstream := &v1alpha1.LDAPIdentityProvider{
ObjectMeta: metav1.ObjectMeta{Name: testName, Namespace: testNamespace, Generation: 1234},
Spec: v1alpha1.LDAPIdentityProviderSpec{
Host: testHost,
TLS: &v1alpha1.LDAPIdentityProviderTLSSpec{CertificateAuthorityData: testCABundle},
Bind: v1alpha1.LDAPIdentityProviderBindSpec{SecretName: testSecretName},
UserSearch: v1alpha1.LDAPIdentityProviderUserSearchSpec{
Base: testUserSearchBase,
Filter: testUserSearchFilter,
Attributes: v1alpha1.LDAPIdentityProviderUserSearchAttributesSpec{
Username: testUsernameAttrName,
UniqueID: testUIDAttrName,
},
},
},
}
providerForValidUpstream := &upstreamldap.Provider{
Name: testName,
Host: testHost,
CABundle: []byte(testCABundle),
BindUsername: testBindUsername,
BindPassword: testBindPassword,
UserSearch: &upstreamldap.UserSearch{
Base: testUserSearchBase,
Filter: testUserSearchFilter,
UsernameAttribute: testUsernameAttrName,
UIDAttribute: testUIDAttrName,
},
Dialer: successfulDialer, // the dialer passed to the controller's constructor should have been passed through
}
tests := []struct { tests := []struct {
name string name string
inputUpstreams []runtime.Object inputUpstreams []runtime.Object
@ -178,48 +213,106 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
{ {
name: "one valid upstream updates the cache to include only that upstream", name: "one valid upstream updates the cache to include only that upstream",
ldapDialer: successfulDialer, ldapDialer: successfulDialer,
inputUpstreams: []runtime.Object{&v1alpha1.LDAPIdentityProvider{ inputUpstreams: []runtime.Object{validUpstream},
ObjectMeta: metav1.ObjectMeta{Name: testName, Namespace: testNamespace, Generation: 1234},
Spec: v1alpha1.LDAPIdentityProviderSpec{
Host: testHost,
TLS: &v1alpha1.LDAPIdentityProviderTLSSpec{CertificateAuthorityData: testCABundle},
Bind: v1alpha1.LDAPIdentityProviderBindSpec{SecretName: testSecretName},
UserSearch: v1alpha1.LDAPIdentityProviderUserSearchSpec{
Base: testUserSearchBase,
Filter: testUserSearchFilter,
Attributes: v1alpha1.LDAPIdentityProviderUserSearchAttributesSpec{
Username: testUsernameAttrName,
UniqueID: testUIDAttrName,
},
},
},
}},
inputSecrets: []runtime.Object{&corev1.Secret{ inputSecrets: []runtime.Object{&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: testSecretName, Namespace: testNamespace}, ObjectMeta: metav1.ObjectMeta{Name: testSecretName, Namespace: testNamespace},
Type: corev1.SecretTypeBasicAuth, Type: corev1.SecretTypeBasicAuth,
Data: testValidSecretData, Data: testValidSecretData,
}}, }},
wantResultingCache: []*upstreamldap.Provider{ wantResultingCache: []*upstreamldap.Provider{providerForValidUpstream},
{
Name: testName,
Host: testHost,
CABundle: []byte(testCABundle),
BindUsername: testBindUsername,
BindPassword: testBindPassword,
UserSearch: &upstreamldap.UserSearch{
Base: testUserSearchBase,
Filter: testUserSearchFilter,
UsernameAttribute: testUsernameAttrName,
UIDAttribute: testUIDAttrName,
},
Dialer: successfulDialer, // the dialer passed to the controller's constructor should have been passed through
},
},
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{ wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234}, ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
Status: v1alpha1.LDAPIdentityProviderStatus{ Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Ready", Phase: "Ready",
// TODO Conditions Conditions: []v1alpha1.Condition{
{
Type: "BindSecretValid",
Status: "True",
LastTransitionTime: now,
Reason: "Success",
Message: "loaded bind secret",
ObservedGeneration: 1234,
},
},
},
}},
},
{
name: "missing secret",
ldapDialer: successfulDialer,
inputUpstreams: []runtime.Object{validUpstream},
inputSecrets: []runtime.Object{},
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
wantResultingCache: []*upstreamldap.Provider{},
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
{
Type: "BindSecretValid",
Status: "False",
LastTransitionTime: now,
Reason: "SecretNotFound",
Message: fmt.Sprintf(`secret "%s" not found`, testSecretName),
ObservedGeneration: 1234,
},
},
},
}},
},
{
name: "secret has wrong type",
ldapDialer: successfulDialer,
inputUpstreams: []runtime.Object{validUpstream},
inputSecrets: []runtime.Object{&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: testSecretName, Namespace: testNamespace},
Type: "some-other-type",
Data: testValidSecretData,
}},
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
wantResultingCache: []*upstreamldap.Provider{},
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
{
Type: "BindSecretValid",
Status: "False",
LastTransitionTime: now,
Reason: "SecretWrongType",
Message: fmt.Sprintf(`referenced Secret "%s" has wrong type "some-other-type" (should be "kubernetes.io/basic-auth")`, testSecretName),
ObservedGeneration: 1234,
},
},
},
}},
},
{
name: "secret is missing key",
ldapDialer: successfulDialer,
inputUpstreams: []runtime.Object{validUpstream},
inputSecrets: []runtime.Object{&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: testSecretName, Namespace: testNamespace},
Type: corev1.SecretTypeBasicAuth,
}},
wantErr: controllerlib.ErrSyntheticRequeue.Error(),
wantResultingCache: []*upstreamldap.Provider{},
wantResultingUpstreams: []v1alpha1.LDAPIdentityProvider{{
ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
Status: v1alpha1.LDAPIdentityProviderStatus{
Phase: "Error",
Conditions: []v1alpha1.Condition{
{
Type: "BindSecretValid",
Status: "False",
LastTransitionTime: now,
Reason: "SecretMissingKeys",
Message: fmt.Sprintf(`referenced Secret "%s" is missing required keys ["username" "password"]`, testSecretName),
ObservedGeneration: 1234,
},
},
}, },
}}, }},
}, },
@ -271,9 +364,13 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
actualUpstreams, err := fakePinnipedClient.IDPV1alpha1().LDAPIdentityProviders(testNamespace).List(ctx, metav1.ListOptions{}) actualUpstreams, err := fakePinnipedClient.IDPV1alpha1().LDAPIdentityProviders(testNamespace).List(ctx, metav1.ListOptions{})
require.NoError(t, err) require.NoError(t, err)
// TODO maybe use something like the normalizeUpstreams() helper to make assertions about what was updated // Assert on the expected Status of the upstreams. Preprocess the upstreams a bit so that they're easier to assert against.
_ = actualUpstreams normalizedActualUpstreams := normalizeLDAPUpstreams(actualUpstreams.Items, now)
// require.ElementsMatch(t, tt.wantResultingUpstreams, actualUpstreams.Items) require.Equal(t, len(tt.wantResultingUpstreams), len(normalizedActualUpstreams))
for i := range tt.wantResultingUpstreams {
// Require each separately to get a nice diff when the test fails.
require.Equal(t, tt.wantResultingUpstreams[i], normalizedActualUpstreams[i])
}
// Running the sync() a second time should be idempotent, and should return the same error. // Running the sync() a second time should be idempotent, and should return the same error.
if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantErr != "" { if err := controllerlib.TestSync(t, controller, syncCtx); tt.wantErr != "" {
@ -284,3 +381,24 @@ func TestLDAPUpstreamWatcherControllerSync(t *testing.T) {
}) })
} }
} }
func normalizeLDAPUpstreams(upstreams []v1alpha1.LDAPIdentityProvider, now metav1.Time) []v1alpha1.LDAPIdentityProvider {
result := make([]v1alpha1.LDAPIdentityProvider, 0, len(upstreams))
for _, u := range upstreams {
normalized := u.DeepCopy()
// We're only interested in comparing the status, so zero out the spec.
normalized.Spec = v1alpha1.LDAPIdentityProviderSpec{}
// Round down the LastTransitionTime values to `now` if they were just updated. This makes
// it much easier to encode assertions about the expected timestamps.
for i := range normalized.Status.Conditions {
if time.Since(normalized.Status.Conditions[i].LastTransitionTime.Time) < 5*time.Second {
normalized.Status.Conditions[i].LastTransitionTime = now
}
}
result = append(result, *normalized)
}
return result
}

View File

@ -338,23 +338,12 @@ func (c *oidcWatcherController) updateStatus(ctx context.Context, upstream *v1al
log := c.log.WithValues("namespace", upstream.Namespace, "name", upstream.Name) log := c.log.WithValues("namespace", upstream.Namespace, "name", upstream.Name)
updated := upstream.DeepCopy() updated := upstream.DeepCopy()
updated.Status.Phase = v1alpha1.PhaseReady hadErrorCondition := mergeConditions(conditions, upstream.Generation, &updated.Status.Conditions, log)
for i := range conditions { updated.Status.Phase = v1alpha1.PhaseReady
cond := conditions[i].DeepCopy() if hadErrorCondition {
cond.LastTransitionTime = metav1.Now()
cond.ObservedGeneration = upstream.Generation
if c.mergeCondition(&updated.Status.Conditions, cond) {
log.Info("updated condition", "type", cond.Type, "status", cond.Status, "reason", cond.Reason, "message", cond.Message)
}
if cond.Status == v1alpha1.ConditionFalse {
updated.Status.Phase = v1alpha1.PhaseError updated.Status.Phase = v1alpha1.PhaseError
} }
}
sort.SliceStable(updated.Status.Conditions, func(i, j int) bool {
return updated.Status.Conditions[i].Type < updated.Status.Conditions[j].Type
})
if equality.Semantic.DeepEqual(upstream, updated) { if equality.Semantic.DeepEqual(upstream, updated) {
return return
@ -369,9 +358,29 @@ func (c *oidcWatcherController) updateStatus(ctx context.Context, upstream *v1al
} }
} }
// mergeConditions merges conditions into conditionsToUpdate. If returns true if it merged any error conditions.
func mergeConditions(conditions []*v1alpha1.Condition, observedGeneration int64, conditionsToUpdate *[]v1alpha1.Condition, log logr.Logger) bool {
hadErrorCondition := false
for i := range conditions {
cond := conditions[i].DeepCopy()
cond.LastTransitionTime = metav1.Now()
cond.ObservedGeneration = observedGeneration
if mergeCondition(conditionsToUpdate, cond) {
log.Info("updated condition", "type", cond.Type, "status", cond.Status, "reason", cond.Reason, "message", cond.Message)
}
if cond.Status == v1alpha1.ConditionFalse {
hadErrorCondition = true
}
}
sort.SliceStable(*conditionsToUpdate, func(i, j int) bool {
return (*conditionsToUpdate)[i].Type < (*conditionsToUpdate)[j].Type
})
return hadErrorCondition
}
// mergeCondition merges a new v1alpha1.Condition into a slice of existing conditions. It returns true // mergeCondition merges a new v1alpha1.Condition into a slice of existing conditions. It returns true
// if the condition has meaningfully changed. // if the condition has meaningfully changed.
func (*oidcWatcherController) mergeCondition(existing *[]v1alpha1.Condition, new *v1alpha1.Condition) bool { func mergeCondition(existing *[]v1alpha1.Condition, new *v1alpha1.Condition) bool {
// Find any existing condition with a matching type. // Find any existing condition with a matching type.
var old *v1alpha1.Condition var old *v1alpha1.Condition
for i := range *existing { for i := range *existing {

View File

@ -655,8 +655,8 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
actualUpstreams, err := fakePinnipedClient.IDPV1alpha1().OIDCIdentityProviders(testNamespace).List(ctx, metav1.ListOptions{}) actualUpstreams, err := fakePinnipedClient.IDPV1alpha1().OIDCIdentityProviders(testNamespace).List(ctx, metav1.ListOptions{})
require.NoError(t, err) require.NoError(t, err)
// Preprocess the set of upstreams a bit so that they're easier to assert against. // Assert on the expected Status of the upstreams. Preprocess the upstreams a bit so that they're easier to assert against.
require.ElementsMatch(t, tt.wantResultingUpstreams, normalizeUpstreams(actualUpstreams.Items, now)) require.ElementsMatch(t, tt.wantResultingUpstreams, normalizeOIDCUpstreams(actualUpstreams.Items, now))
// Running the sync() a second time should be idempotent except for logs, and should return the same error. // Running the sync() a second time should be idempotent except for logs, and should return the same error.
// This also helps exercise code paths where the OIDC provider discovery hits cache. // This also helps exercise code paths where the OIDC provider discovery hits cache.
@ -669,7 +669,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
} }
} }
func normalizeUpstreams(upstreams []v1alpha1.OIDCIdentityProvider, now metav1.Time) []v1alpha1.OIDCIdentityProvider { func normalizeOIDCUpstreams(upstreams []v1alpha1.OIDCIdentityProvider, now metav1.Time) []v1alpha1.OIDCIdentityProvider {
result := make([]v1alpha1.OIDCIdentityProvider, 0, len(upstreams)) result := make([]v1alpha1.OIDCIdentityProvider, 0, len(upstreams))
for _, u := range upstreams { for _, u := range upstreams {
normalized := u.DeepCopy() normalized := u.DeepCopy()