Status condition messages for IDP transforms show index of invalid IDP

This commit is contained in:
Ryan Richard 2023-07-18 15:00:58 -07:00
parent b89e6d9d93
commit e42e3ca421
2 changed files with 453 additions and 156 deletions

View File

@ -328,7 +328,7 @@ func (c *federationDomainWatcherController) makeLegacyFederationDomainIssuer(
conditions = appendIdentityProviderDuplicateDisplayNamesCondition(sets.Set[string]{}, conditions) conditions = appendIdentityProviderDuplicateDisplayNamesCondition(sets.Set[string]{}, conditions)
conditions = appendIdentityProviderObjectRefAPIGroupSuffixCondition(c.apiGroup, []string{}, conditions) conditions = appendIdentityProviderObjectRefAPIGroupSuffixCondition(c.apiGroup, []string{}, conditions)
conditions = appendIdentityProviderObjectRefKindCondition(c.sortedAllowedKinds(), []string{}, conditions) conditions = appendIdentityProviderObjectRefKindCondition(c.sortedAllowedKinds(), []string{}, conditions)
conditions = appendTransformsConstantsNamesUniqueCondition(sets.Set[string]{}, conditions) conditions = appendTransformsConstantsNamesUniqueCondition([]string{}, conditions)
conditions = appendTransformsExpressionsValidCondition([]string{}, conditions) conditions = appendTransformsExpressionsValidCondition([]string{}, conditions)
conditions = appendTransformsExamplesPassedCondition([]string{}, conditions) conditions = appendTransformsExamplesPassedCondition([]string{}, conditions)
@ -347,6 +347,7 @@ func (c *federationDomainWatcherController) makeFederationDomainIssuerWithExplic
duplicateDisplayNames := sets.Set[string]{} duplicateDisplayNames := sets.Set[string]{}
badAPIGroupNames := []string{} badAPIGroupNames := []string{}
badKinds := []string{} badKinds := []string{}
validationErrorMessages := &transformsValidationErrorMessages{}
for index, idp := range federationDomain.Spec.IdentityProviders { for index, idp := range federationDomain.Spec.IdentityProviders {
idpIsValid := true idpIsValid := true
@ -397,7 +398,8 @@ func (c *federationDomainWatcherController) makeFederationDomainIssuerWithExplic
var err error var err error
var pipeline *idtransform.TransformationPipeline var pipeline *idtransform.TransformationPipeline
var allExamplesPassed bool var allExamplesPassed bool
pipeline, allExamplesPassed, conditions, err = c.makeTransformationPipelineAndEvaluateExamplesForIdentityProvider(ctx, idp, index, conditions) pipeline, allExamplesPassed, err = c.makeTransformationPipelineAndEvaluateExamplesForIdentityProvider(
ctx, idp, index, validationErrorMessages)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -448,6 +450,10 @@ func (c *federationDomainWatcherController) makeFederationDomainIssuerWithExplic
conditions = appendIdentityProviderObjectRefAPIGroupSuffixCondition(c.apiGroup, badAPIGroupNames, conditions) conditions = appendIdentityProviderObjectRefAPIGroupSuffixCondition(c.apiGroup, badAPIGroupNames, conditions)
conditions = appendIdentityProviderObjectRefKindCondition(c.sortedAllowedKinds(), badKinds, conditions) conditions = appendIdentityProviderObjectRefKindCondition(c.sortedAllowedKinds(), badKinds, conditions)
conditions = appendTransformsConstantsNamesUniqueCondition(validationErrorMessages.errorsForConstants, conditions)
conditions = appendTransformsExpressionsValidCondition(validationErrorMessages.errorsForExpressions, conditions)
conditions = appendTransformsExamplesPassedCondition(validationErrorMessages.errorsForExamples, conditions)
return federationDomainIssuer, conditions, nil return federationDomainIssuer, conditions, nil
} }
func (c *federationDomainWatcherController) findIDPsUIDByObjectRef(objectRef corev1.TypedLocalObjectReference, namespace string) (types.UID, bool, error) { func (c *federationDomainWatcherController) findIDPsUIDByObjectRef(objectRef corev1.TypedLocalObjectReference, namespace string) (types.UID, bool, error) {
@ -482,27 +488,36 @@ func (c *federationDomainWatcherController) makeTransformationPipelineAndEvaluat
ctx context.Context, ctx context.Context,
idp configv1alpha1.FederationDomainIdentityProvider, idp configv1alpha1.FederationDomainIdentityProvider,
idpIndex int, idpIndex int,
conditions []*configv1alpha1.Condition, validationErrorMessages *transformsValidationErrorMessages,
) (*idtransform.TransformationPipeline, bool, []*configv1alpha1.Condition, error) { ) (*idtransform.TransformationPipeline, bool, error) {
consts, conditions, err := c.makeTransformsConstants(idp, conditions) consts, errorsForConstants, err := c.makeTransformsConstantsForIdentityProvider(idp, idpIndex)
if err != nil { if err != nil {
return nil, false, nil, err return nil, false, err
}
if len(errorsForConstants) > 0 {
validationErrorMessages.errorsForConstants = append(validationErrorMessages.errorsForConstants, errorsForConstants)
} }
pipeline, conditions, err := c.makeTransformationPipeline(idp, idpIndex, consts, conditions) pipeline, errorsForExpressions, err := c.makeTransformationPipelineForIdentityProvider(idp, idpIndex, consts)
if err != nil { if err != nil {
return nil, false, nil, err return nil, false, err
}
if len(errorsForExpressions) > 0 {
validationErrorMessages.errorsForExpressions = append(validationErrorMessages.errorsForExpressions, errorsForExpressions)
} }
allExamplesPassed, conditions := c.evaluateExamples(ctx, idp, idpIndex, pipeline, conditions) allExamplesPassed, errorsForExamples := c.evaluateExamplesForIdentityProvider(ctx, idp, idpIndex, pipeline)
if len(errorsForExamples) > 0 {
validationErrorMessages.errorsForExamples = append(validationErrorMessages.errorsForExamples, errorsForExamples)
}
return pipeline, allExamplesPassed, conditions, nil return pipeline, allExamplesPassed, nil
} }
func (c *federationDomainWatcherController) makeTransformsConstants( func (c *federationDomainWatcherController) makeTransformsConstantsForIdentityProvider(
idp configv1alpha1.FederationDomainIdentityProvider, idp configv1alpha1.FederationDomainIdentityProvider,
conditions []*configv1alpha1.Condition, idpIndex int,
) (*celtransformer.TransformationConstants, []*configv1alpha1.Condition, error) { ) (*celtransformer.TransformationConstants, string, error) {
consts := &celtransformer.TransformationConstants{ consts := &celtransformer.TransformationConstants{
StringConstants: map[string]string{}, StringConstants: map[string]string{},
StringListConstants: map[string][]string{}, StringListConstants: map[string][]string{},
@ -525,21 +540,24 @@ func (c *federationDomainWatcherController) makeTransformsConstants(
consts.StringListConstants[constant.Name] = constant.StringListValue consts.StringListConstants[constant.Name] = constant.StringListValue
default: default:
// This shouldn't really happen since the CRD validates it, but handle it as an error. // This shouldn't really happen since the CRD validates it, but handle it as an error.
return nil, nil, fmt.Errorf("one of spec.identityProvider[].transforms.constants[].type is invalid: %q", constant.Type) return nil, "", fmt.Errorf("one of spec.identityProvider[].transforms.constants[].type is invalid: %q", constant.Type)
} }
} }
conditions = appendTransformsConstantsNamesUniqueCondition(duplicateConstNames, conditions) if duplicateConstNames.Len() > 0 {
return consts, fmt.Sprintf(
"the names specified by .spec.identityProviders[%d].transforms.constants[].name contain duplicates: %s",
idpIndex, strings.Join(sortAndQuote(duplicateConstNames.UnsortedList()), ", ")), nil
}
return consts, conditions, nil return consts, "", nil
} }
func (c *federationDomainWatcherController) makeTransformationPipeline( func (c *federationDomainWatcherController) makeTransformationPipelineForIdentityProvider(
idp configv1alpha1.FederationDomainIdentityProvider, idp configv1alpha1.FederationDomainIdentityProvider,
idpIndex int, idpIndex int,
consts *celtransformer.TransformationConstants, consts *celtransformer.TransformationConstants,
conditions []*configv1alpha1.Condition, ) (*idtransform.TransformationPipeline, string, error) {
) (*idtransform.TransformationPipeline, []*configv1alpha1.Condition, error) {
pipeline := idtransform.NewTransformationPipeline() pipeline := idtransform.NewTransformationPipeline()
expressionsCompileErrors := []string{} expressionsCompileErrors := []string{}
@ -558,7 +576,7 @@ func (c *federationDomainWatcherController) makeTransformationPipeline(
} }
default: default:
// This shouldn't really happen since the CRD validates it, but handle it as an error. // This shouldn't really happen since the CRD validates it, but handle it as an error.
return nil, nil, fmt.Errorf("one of spec.identityProvider[].transforms.expressions[].type is invalid: %q", expr.Type) return nil, "", fmt.Errorf("one of spec.identityProvider[].transforms.expressions[].type is invalid: %q", expr.Type)
} }
compiledTransform, err := c.celTransformer.CompileTransformation(rawTransform, consts) compiledTransform, err := c.celTransformer.CompileTransformation(rawTransform, consts)
@ -571,30 +589,29 @@ func (c *federationDomainWatcherController) makeTransformationPipeline(
pipeline.AppendTransformation(compiledTransform) pipeline.AppendTransformation(compiledTransform)
} }
conditions = appendTransformsExpressionsValidCondition(expressionsCompileErrors, conditions)
if len(expressionsCompileErrors) > 0 { if len(expressionsCompileErrors) > 0 {
// One or more of the expressions did not compile, so we don't have a useful pipeline to return. // One or more of the expressions did not compile, so we don't have a useful pipeline to return.
return nil, conditions, nil // Return the validation messages.
return nil, strings.Join(expressionsCompileErrors, "\n\n"), nil
} }
return pipeline, conditions, nil return pipeline, "", nil
} }
func (c *federationDomainWatcherController) evaluateExamples( func (c *federationDomainWatcherController) evaluateExamplesForIdentityProvider(
ctx context.Context, ctx context.Context,
idp configv1alpha1.FederationDomainIdentityProvider, idp configv1alpha1.FederationDomainIdentityProvider,
idpIndex int, idpIndex int,
pipeline *idtransform.TransformationPipeline, pipeline *idtransform.TransformationPipeline,
conditions []*configv1alpha1.Condition, ) (bool, string) {
) (bool, []*configv1alpha1.Condition) {
const errorFmt = ".spec.identityProviders[%d].transforms.examples[%d] example failed:\nexpected: %s\nactual: %s" const errorFmt = ".spec.identityProviders[%d].transforms.examples[%d] example failed:\nexpected: %s\nactual: %s"
examplesErrors := []string{} examplesErrors := []string{}
if pipeline == nil { if pipeline == nil {
// Unable to evaluate the conditions where the pipeline of expressions was invalid. // Unable to evaluate the conditions where the pipeline of expressions was invalid.
conditions = appendTransformsExamplesPassedCondition(nil, conditions) return false, fmt.Sprintf(
return false, conditions "unable to check if the examples specified by .spec.identityProviders[%d].transforms.examples[] had errors because an expression was invalid",
idpIndex)
} }
// Run all the provided transform examples. If any fail, put errors on the FederationDomain status. // Run all the provided transform examples. If any fail, put errors on the FederationDomain status.
@ -652,13 +669,11 @@ func (c *federationDomainWatcherController) evaluateExamples(
} }
} }
conditions = appendTransformsExamplesPassedCondition(examplesErrors, conditions) if len(examplesErrors) > 0 {
return false, strings.Join(examplesErrors, "\n\n")
}
return len(examplesErrors) == 0, conditions return true, ""
}
func (c *federationDomainWatcherController) sortedAllowedKinds() []string {
return sortAndQuote(c.allowedKinds.UnsortedList())
} }
func appendIdentityProviderObjectRefKindCondition(expectedKinds []string, badSuffixNames []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition { func appendIdentityProviderObjectRefKindCondition(expectedKinds []string, badSuffixNames []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
@ -704,11 +719,10 @@ func appendIdentityProviderObjectRefAPIGroupSuffixCondition(expectedSuffixName s
func appendTransformsExpressionsValidCondition(errors []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition { func appendTransformsExpressionsValidCondition(errors []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
if len(errors) > 0 { if len(errors) > 0 {
conditions = append(conditions, &configv1alpha1.Condition{ conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsExpressionsValid, Type: typeTransformsExpressionsValid,
Status: configv1alpha1.ConditionFalse, Status: configv1alpha1.ConditionFalse,
Reason: reasonInvalidTransformsExpressions, Reason: reasonInvalidTransformsExpressions,
Message: fmt.Sprintf("the expressions specified by .spec.identityProviders[].transforms.expressions[] were invalid:\n\n%s", Message: strings.Join(errors, "\n\n"),
strings.Join(errors, "\n\n")),
}) })
} else { } else {
conditions = append(conditions, &configv1alpha1.Condition{ conditions = append(conditions, &configv1alpha1.Condition{
@ -722,23 +736,14 @@ func appendTransformsExpressionsValidCondition(errors []string, conditions []*co
} }
func appendTransformsExamplesPassedCondition(errors []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition { func appendTransformsExamplesPassedCondition(errors []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
switch { if len(errors) > 0 {
case errors == nil:
conditions = append(conditions, &configv1alpha1.Condition{ conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsExamplesPassed, Type: typeTransformsExamplesPassed,
Status: configv1alpha1.ConditionUnknown, Status: configv1alpha1.ConditionFalse,
Reason: reasonUnableToValidate, Reason: reasonTransformsExamplesFailed,
Message: "unable to check if the examples specified by .spec.identityProviders[].transforms.examples[] had errors because an expression was invalid", Message: strings.Join(errors, "\n\n"),
}) })
case len(errors) > 0: } else {
conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsExamplesPassed,
Status: configv1alpha1.ConditionFalse,
Reason: reasonTransformsExamplesFailed,
Message: fmt.Sprintf("the examples specified by .spec.identityProviders[].transforms.examples[] had errors:\n\n%s",
strings.Join(errors, "\n\n")),
})
default:
conditions = append(conditions, &configv1alpha1.Condition{ conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsExamplesPassed, Type: typeTransformsExamplesPassed,
Status: configv1alpha1.ConditionTrue, Status: configv1alpha1.ConditionTrue,
@ -749,6 +754,25 @@ func appendTransformsExamplesPassedCondition(errors []string, conditions []*conf
return conditions return conditions
} }
func appendTransformsConstantsNamesUniqueCondition(errors []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
if len(errors) > 0 {
conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsConstantsNamesUnique,
Status: configv1alpha1.ConditionFalse,
Reason: reasonDuplicateConstantsNames,
Message: strings.Join(errors, "\n\n"),
})
} else {
conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsConstantsNamesUnique,
Status: configv1alpha1.ConditionTrue,
Reason: reasonSuccess,
Message: "the names specified by .spec.identityProviders[].transforms.constants[].name are unique",
})
}
return conditions
}
func appendIdentityProviderDuplicateDisplayNamesCondition(duplicateDisplayNames sets.Set[string], conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition { func appendIdentityProviderDuplicateDisplayNamesCondition(duplicateDisplayNames sets.Set[string], conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
if duplicateDisplayNames.Len() > 0 { if duplicateDisplayNames.Len() > 0 {
conditions = append(conditions, &configv1alpha1.Condition{ conditions = append(conditions, &configv1alpha1.Condition{
@ -769,26 +793,6 @@ func appendIdentityProviderDuplicateDisplayNamesCondition(duplicateDisplayNames
return conditions return conditions
} }
func appendTransformsConstantsNamesUniqueCondition(duplicateConstNames sets.Set[string], conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
if duplicateConstNames.Len() > 0 {
conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsConstantsNamesUnique,
Status: configv1alpha1.ConditionFalse,
Reason: reasonDuplicateConstantsNames,
Message: fmt.Sprintf("the names specified by .spec.identityProviders[].transforms.constants[].name contain duplicates: %s",
strings.Join(sortAndQuote(duplicateConstNames.UnsortedList()), ", ")),
})
} else {
conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsConstantsNamesUnique,
Status: configv1alpha1.ConditionTrue,
Reason: reasonSuccess,
Message: "the names specified by .spec.identityProviders[].transforms.constants[].name are unique",
})
}
return conditions
}
func appendIssuerURLValidCondition(err error, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition { func appendIssuerURLValidCondition(err error, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
if err != nil { if err != nil {
// Note that the FederationDomainIssuer constructors only validate the Issuer URL, // Note that the FederationDomainIssuer constructors only validate the Issuer URL,
@ -810,15 +814,6 @@ func appendIssuerURLValidCondition(err error, conditions []*configv1alpha1.Condi
return conditions return conditions
} }
func sortAndQuote(strs []string) []string {
quoted := make([]string, 0, len(strs))
for _, s := range strs {
quoted = append(quoted, fmt.Sprintf("%q", s))
}
sort.Strings(quoted)
return quoted
}
func (c *federationDomainWatcherController) updateStatus( func (c *federationDomainWatcherController) updateStatus(
ctx context.Context, ctx context.Context,
federationDomain *configv1alpha1.FederationDomain, federationDomain *configv1alpha1.FederationDomain,
@ -859,6 +854,25 @@ func (c *federationDomainWatcherController) updateStatus(
return err return err
} }
func sortAndQuote(strs []string) []string {
quoted := make([]string, 0, len(strs))
for _, s := range strs {
quoted = append(quoted, fmt.Sprintf("%q", s))
}
sort.Strings(quoted)
return quoted
}
func (c *federationDomainWatcherController) sortedAllowedKinds() []string {
return sortAndQuote(c.allowedKinds.UnsortedList())
}
type transformsValidationErrorMessages struct {
errorsForConstants []string
errorsForExpressions []string
errorsForExamples []string
}
type crossFederationDomainConfigValidator struct { type crossFederationDomainConfigValidator struct {
issuerCounts map[string]int issuerCounts map[string]int
uniqueSecretNamesPerIssuerAddress map[string]map[string]bool uniqueSecretNamesPerIssuerAddress map[string]map[string]bool

View File

@ -411,14 +411,14 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
} }
} }
sadConstNamesUniqueCondition := func(duplicateNames string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition { sadConstNamesUniqueCondition := func(errorMessages string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
return configv1alpha1.Condition{ return configv1alpha1.Condition{
Type: "TransformsConstantsNamesUnique", Type: "TransformsConstantsNamesUnique",
Status: "False", Status: "False",
ObservedGeneration: observedGeneration, ObservedGeneration: observedGeneration,
LastTransitionTime: time, LastTransitionTime: time,
Reason: "DuplicateConstantsNames", Reason: "DuplicateConstantsNames",
Message: fmt.Sprintf("the names specified by .spec.identityProviders[].transforms.constants[].name contain duplicates: %s", duplicateNames), Message: errorMessages,
} }
} }
@ -440,7 +440,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
ObservedGeneration: observedGeneration, ObservedGeneration: observedGeneration,
LastTransitionTime: time, LastTransitionTime: time,
Reason: "InvalidTransformsExpressions", Reason: "InvalidTransformsExpressions",
Message: fmt.Sprintf("the expressions specified by .spec.identityProviders[].transforms.expressions[] were invalid:\n\n%s", errorMessages), Message: errorMessages,
} }
} }
@ -462,18 +462,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
ObservedGeneration: observedGeneration, ObservedGeneration: observedGeneration,
LastTransitionTime: time, LastTransitionTime: time,
Reason: "TransformsExamplesFailed", Reason: "TransformsExamplesFailed",
Message: fmt.Sprintf("the examples specified by .spec.identityProviders[].transforms.examples[] had errors:\n\n%s", errorMessages), Message: errorMessages,
}
}
unknownTransformationExamplesCondition := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "TransformsExamplesPassed",
Status: "Unknown",
ObservedGeneration: observedGeneration,
LastTransitionTime: time,
Reason: "UnableToValidate",
Message: "unable to check if the examples specified by .spec.identityProviders[].transforms.examples[] had errors because an expression was invalid",
} }
} }
@ -587,7 +576,7 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
wantFDIssuers: []*federationdomainproviders.FederationDomainIssuer{}, wantFDIssuers: []*federationdomainproviders.FederationDomainIssuer{},
}, },
{ {
name: "legacy config: when no identity provider is specified on federation domains, but exactly one identity " + name: "legacy config: when no identity provider is specified on federation domains, but exactly one OIDC identity " +
"provider resource exists on cluster, the controller will set a default IDP on each federation domain " + "provider resource exists on cluster, the controller will set a default IDP on each federation domain " +
"matching the only identity provider found", "matching the only identity provider found",
inputObjects: []runtime.Object{ inputObjects: []runtime.Object{
@ -610,6 +599,54 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
), ),
}, },
}, },
{
name: "legacy config: when no identity provider is specified on federation domains, but exactly one LDAP identity " +
"provider resource exists on cluster, the controller will set a default IDP on each federation domain " +
"matching the only identity provider found",
inputObjects: []runtime.Object{
federationDomain1,
federationDomain2,
ldapIdentityProvider,
},
wantFDIssuers: []*federationdomainproviders.FederationDomainIssuer{
federationDomainIssuerWithDefaultIDP(t, federationDomain1.Spec.Issuer, ldapIdentityProvider.ObjectMeta),
federationDomainIssuerWithDefaultIDP(t, federationDomain2.Spec.Issuer, ldapIdentityProvider.ObjectMeta),
},
wantStatusUpdates: []*configv1alpha1.FederationDomain{
expectedFederationDomainStatusUpdate(federationDomain1,
configv1alpha1.FederationDomainPhaseReady,
allHappyConditionsLegacyConfigurationSuccess(federationDomain1.Spec.Issuer, ldapIdentityProvider.Name, frozenMetav1Now, 123),
),
expectedFederationDomainStatusUpdate(federationDomain2,
configv1alpha1.FederationDomainPhaseReady,
allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, ldapIdentityProvider.Name, frozenMetav1Now, 123),
),
},
},
{
name: "legacy config: when no identity provider is specified on federation domains, but exactly one AD identity " +
"provider resource exists on cluster, the controller will set a default IDP on each federation domain " +
"matching the only identity provider found",
inputObjects: []runtime.Object{
federationDomain1,
federationDomain2,
adIdentityProvider,
},
wantFDIssuers: []*federationdomainproviders.FederationDomainIssuer{
federationDomainIssuerWithDefaultIDP(t, federationDomain1.Spec.Issuer, adIdentityProvider.ObjectMeta),
federationDomainIssuerWithDefaultIDP(t, federationDomain2.Spec.Issuer, adIdentityProvider.ObjectMeta),
},
wantStatusUpdates: []*configv1alpha1.FederationDomain{
expectedFederationDomainStatusUpdate(federationDomain1,
configv1alpha1.FederationDomainPhaseReady,
allHappyConditionsLegacyConfigurationSuccess(federationDomain1.Spec.Issuer, adIdentityProvider.Name, frozenMetav1Now, 123),
),
expectedFederationDomainStatusUpdate(federationDomain2,
configv1alpha1.FederationDomainPhaseReady,
allHappyConditionsLegacyConfigurationSuccess(federationDomain2.Spec.Issuer, adIdentityProvider.Name, frozenMetav1Now, 123),
),
},
},
{ {
name: "when there are two valid FederationDomains, but one is already up to date, the sync loop only updates " + name: "when there are two valid FederationDomains, but one is already up to date, the sync loop only updates " +
"the out-of-date FederationDomain", "the out-of-date FederationDomain",
@ -637,6 +674,40 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
), ),
}, },
}, },
{
name: "when the status of the FederationDomains is based on an old generation, it is updated",
inputObjects: []runtime.Object{
oidcIdentityProvider,
&configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: federationDomain1.Name, Namespace: federationDomain1.Namespace, Generation: 123},
Spec: configv1alpha1.FederationDomainSpec{Issuer: federationDomain1.Spec.Issuer},
Status: configv1alpha1.FederationDomainStatus{
Phase: configv1alpha1.FederationDomainPhaseReady,
Conditions: allHappyConditionsLegacyConfigurationSuccess(
federationDomain1.Spec.Issuer,
oidcIdentityProvider.Name,
frozenMetav1Now,
2, // this is an older generation
),
},
},
},
wantFDIssuers: []*federationdomainproviders.FederationDomainIssuer{
federationDomainIssuerWithDefaultIDP(t, federationDomain1.Spec.Issuer, oidcIdentityProvider.ObjectMeta),
},
wantStatusUpdates: []*configv1alpha1.FederationDomain{
// only one update, because the other FederationDomain already had the right status
expectedFederationDomainStatusUpdate(federationDomain1,
configv1alpha1.FederationDomainPhaseReady,
allHappyConditionsLegacyConfigurationSuccess(
federationDomain1.Spec.Issuer,
oidcIdentityProvider.Name,
frozenMetav1Now,
123, // all conditions are updated to the new observed generation
),
),
},
},
{ {
name: "when there are two valid FederationDomains, but updating one fails, the status on the FederationDomain will not change", name: "when there are two valid FederationDomains, but updating one fails, the status on the FederationDomain will not change",
inputObjects: []runtime.Object{ inputObjects: []runtime.Object{
@ -1306,7 +1377,9 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
replaceConditions( replaceConditions(
allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123),
[]configv1alpha1.Condition{ []configv1alpha1.Condition{
sadConstNamesUniqueCondition(`"duplicate1", "duplicate2"`, frozenMetav1Now, 123), sadConstNamesUniqueCondition(
`the names specified by .spec.identityProviders[0].transforms.constants[].name contain duplicates: "duplicate1", "duplicate2"`,
frozenMetav1Now, 123),
sadReadyCondition(frozenMetav1Now, 123), sadReadyCondition(frozenMetav1Now, 123),
}), }),
), ),
@ -1351,25 +1424,25 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
replaceConditions( replaceConditions(
allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123),
[]configv1alpha1.Condition{ []configv1alpha1.Condition{
sadTransformationExpressionsCondition( sadTransformationExpressionsCondition(here.Doc(
here.Doc( `spec.identityProvider[0].transforms.expressions[0].expression was invalid:
`spec.identityProvider[0].transforms.expressions[0].expression was invalid: CEL expression compile error: ERROR: <input>:1:6: Syntax error: mismatched input 'is' expecting <EOF>
CEL expression compile error: ERROR: <input>:1:6: Syntax error: mismatched input 'is' expecting <EOF> | this is not a valid cel expression
| this is not a valid cel expression | .....^
| .....^
spec.identityProvider[0].transforms.expressions[1].expression was invalid: spec.identityProvider[0].transforms.expressions[1].expression was invalid:
CEL expression compile error: ERROR: <input>:1:6: Syntax error: mismatched input 'is' expecting <EOF> CEL expression compile error: ERROR: <input>:1:6: Syntax error: mismatched input 'is' expecting <EOF>
| this is also not a valid cel expression | this is also not a valid cel expression
| .....^ | .....^
spec.identityProvider[0].transforms.expressions[3].expression was invalid: spec.identityProvider[0].transforms.expressions[3].expression was invalid:
CEL expression compile error: ERROR: <input>:1:7: Syntax error: mismatched input 'not' expecting <EOF> CEL expression compile error: ERROR: <input>:1:7: Syntax error: mismatched input 'not' expecting <EOF>
| still not a valid cel expression | still not a valid cel expression
| ......^`, | ......^`,
), ), frozenMetav1Now, 123),
sadTransformationExamplesCondition(
"unable to check if the examples specified by .spec.identityProviders[0].transforms.examples[] had errors because an expression was invalid",
frozenMetav1Now, 123), frozenMetav1Now, 123),
unknownTransformationExamplesCondition(frozenMetav1Now, 123),
sadReadyCondition(frozenMetav1Now, 123), sadReadyCondition(frozenMetav1Now, 123),
}), }),
), ),
@ -1497,45 +1570,43 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
replaceConditions( replaceConditions(
allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123),
[]configv1alpha1.Condition{ []configv1alpha1.Condition{
sadTransformationExamplesCondition( sadTransformationExamplesCondition(here.Doc(
here.Doc( `.spec.identityProviders[0].transforms.examples[2] example failed:
`.spec.identityProviders[0].transforms.examples[2] example failed: expected: authentication to be rejected
expected: authentication to be rejected actual: authentication was not rejected
actual: authentication was not rejected
.spec.identityProviders[0].transforms.examples[3] example failed: .spec.identityProviders[0].transforms.examples[3] example failed:
expected: authentication not to be rejected expected: authentication not to be rejected
actual: authentication was rejected with message "only ryan allowed" actual: authentication was rejected with message "only ryan allowed"
.spec.identityProviders[0].transforms.examples[4] example failed: .spec.identityProviders[0].transforms.examples[4] example failed:
expected: authentication rejection message "wrong message" expected: authentication rejection message "wrong message"
actual: authentication rejection message "only ryan allowed" actual: authentication rejection message "only ryan allowed"
.spec.identityProviders[0].transforms.examples[6] example failed: .spec.identityProviders[0].transforms.examples[6] example failed:
expected: username "wrong" expected: username "wrong"
actual: username "pre:ryan" actual: username "pre:ryan"
.spec.identityProviders[0].transforms.examples[6] example failed: .spec.identityProviders[0].transforms.examples[6] example failed:
expected: groups [] expected: groups []
actual: groups ["pre:a", "pre:b"] actual: groups ["pre:a", "pre:b"]
.spec.identityProviders[0].transforms.examples[7] example failed: .spec.identityProviders[0].transforms.examples[7] example failed:
expected: username "wrong" expected: username "wrong"
actual: username "pre:ryan" actual: username "pre:ryan"
.spec.identityProviders[0].transforms.examples[8] example failed: .spec.identityProviders[0].transforms.examples[8] example failed:
expected: groups ["wrong1", "wrong2"] expected: groups ["wrong1", "wrong2"]
actual: groups ["pre:a", "pre:b"] actual: groups ["pre:a", "pre:b"]
.spec.identityProviders[0].transforms.examples[9] example failed: .spec.identityProviders[0].transforms.examples[9] example failed:
expected: username "" expected: username ""
actual: username "pre:ryan" actual: username "pre:ryan"
.spec.identityProviders[0].transforms.examples[9] example failed: .spec.identityProviders[0].transforms.examples[9] example failed:
expected: groups [] expected: groups []
actual: groups ["pre:a", "pre:b"]`, actual: groups ["pre:a", "pre:b"]`,
), ), frozenMetav1Now, 123),
frozenMetav1Now, 123),
sadReadyCondition(frozenMetav1Now, 123), sadReadyCondition(frozenMetav1Now, 123),
}), }),
), ),
@ -1598,16 +1669,228 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
replaceConditions( replaceConditions(
allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123), allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123),
[]configv1alpha1.Condition{ []configv1alpha1.Condition{
sadTransformationExamplesCondition( sadTransformationExamplesCondition(here.Doc(
here.Doc( `.spec.identityProviders[0].transforms.examples[0] example failed:
`.spec.identityProviders[0].transforms.examples[0] example failed: expected: no transformation errors
expected: no transformation errors actual: transformations resulted in an unexpected error "identity transformation returned an empty username, which is not allowed"
actual: transformations resulted in an unexpected error "identity transformation returned an empty username, which is not allowed"
.spec.identityProviders[0].transforms.examples[1] example failed: .spec.identityProviders[0].transforms.examples[1] example failed:
expected: no transformation errors expected: no transformation errors
actual: transformations resulted in an unexpected error "identity transformation returned an empty username, which is not allowed"`, actual: transformations resulted in an unexpected error "identity transformation returned an empty username, which is not allowed"`,
), ), frozenMetav1Now, 123),
sadReadyCondition(frozenMetav1Now, 123),
}),
),
},
},
{
name: "the federation domain has lots of errors including errors from multiple IDPs, which are all shown in the status conditions using IDP indices in the messages",
inputObjects: []runtime.Object{
oidcIdentityProvider,
&configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
Spec: configv1alpha1.FederationDomainSpec{
Issuer: "https://not-unique.com",
IdentityProviders: []configv1alpha1.FederationDomainIdentityProvider{
{
DisplayName: "not unique",
ObjectRef: corev1.TypedLocalObjectReference{
APIGroup: pointer.String(apiGroupSupervisor),
Kind: "OIDCIdentityProvider",
Name: "this will not be found",
},
Transforms: configv1alpha1.FederationDomainTransforms{
Constants: []configv1alpha1.FederationDomainTransformsConstant{
{Name: "foo", Type: "string", StringValue: "bar"},
{Name: "foo", Type: "string", StringValue: "baz"},
},
Expressions: []configv1alpha1.FederationDomainTransformsExpression{
{Type: "username/v1", Expression: `username + ":suffix"`},
},
Examples: []configv1alpha1.FederationDomainTransformsExample{
{ // this should fail
Username: "ryan",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Username: "this is wrong string",
Groups: []string{"this is wrong string list"},
},
},
{ // this should fail
Username: "ryan",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Username: "this is also wrong string",
Groups: []string{"this is also wrong string list"},
},
},
},
},
},
{
DisplayName: "not unique",
ObjectRef: corev1.TypedLocalObjectReference{
APIGroup: pointer.String(apiGroupSupervisor),
Kind: "this is wrong",
Name: "foo",
},
Transforms: configv1alpha1.FederationDomainTransforms{
Constants: []configv1alpha1.FederationDomainTransformsConstant{
{Name: "foo", Type: "string", StringValue: "bar"},
{Name: "foo", Type: "string", StringValue: "baz"},
},
Expressions: []configv1alpha1.FederationDomainTransformsExpression{
{Type: "username/v1", Expression: `username + ":suffix"`},
},
Examples: []configv1alpha1.FederationDomainTransformsExample{
{ // this should pass
Username: "ryan",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Username: "ryan:suffix",
Groups: []string{"a", "b"},
},
},
{ // this should fail
Username: "ryan",
Groups: []string{"a", "b"},
Expects: configv1alpha1.FederationDomainTransformsExampleExpects{
Username: "this is still wrong string",
Groups: []string{"this is still wrong string list"},
},
},
},
},
},
{
DisplayName: "name1",
ObjectRef: corev1.TypedLocalObjectReference{
APIGroup: pointer.String("this is wrong"),
Kind: "OIDCIdentityProvider",
Name: "foo",
},
Transforms: configv1alpha1.FederationDomainTransforms{
Expressions: []configv1alpha1.FederationDomainTransformsExpression{
{Type: "username/v1", Expression: `username`},
{Type: "username/v1", Expression: `this does not compile`},
{Type: "username/v1", Expression: `username`},
{Type: "username/v1", Expression: `this also does not compile`},
},
},
},
},
},
},
&configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "config2", Namespace: namespace, Generation: 123},
Spec: configv1alpha1.FederationDomainSpec{
Issuer: "https://not-unique.com",
IdentityProviders: []configv1alpha1.FederationDomainIdentityProvider{
{
DisplayName: "name1",
ObjectRef: corev1.TypedLocalObjectReference{
APIGroup: pointer.String(apiGroupSupervisor),
Kind: "OIDCIdentityProvider",
Name: oidcIdentityProvider.Name,
},
Transforms: configv1alpha1.FederationDomainTransforms{
Expressions: []configv1alpha1.FederationDomainTransformsExpression{
{Type: "username/v1", Expression: `username`},
{Type: "username/v1", Expression: `this still does not compile`},
{Type: "username/v1", Expression: `username`},
{Type: "username/v1", Expression: `this really does not compile`},
},
},
},
},
},
},
},
wantFDIssuers: []*federationdomainproviders.FederationDomainIssuer{},
wantStatusUpdates: []*configv1alpha1.FederationDomain{
expectedFederationDomainStatusUpdate(
&configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
},
configv1alpha1.FederationDomainPhaseError,
replaceConditions(
allHappyConditionsSuccess("https://not-unique.com", frozenMetav1Now, 123),
[]configv1alpha1.Condition{
sadConstNamesUniqueCondition(here.Doc(
`the names specified by .spec.identityProviders[0].transforms.constants[].name contain duplicates: "foo"
the names specified by .spec.identityProviders[1].transforms.constants[].name contain duplicates: "foo"`,
), frozenMetav1Now, 123),
sadAPIGroupSuffixCondition(`"this is wrong"`, frozenMetav1Now, 123),
sadDisplayNamesUniqueCondition(`"not unique"`, frozenMetav1Now, 123),
sadIdentityProvidersFoundConditionIdentityProvidersObjectRefsNotFound(
`.spec.identityProviders[0] with displayName "not unique", .spec.identityProviders[1] with displayName "not unique", .spec.identityProviders[2] with displayName "name1"`,
frozenMetav1Now, 123),
sadIssuerIsUniqueCondition(frozenMetav1Now, 123),
sadKindCondition(`"this is wrong"`, frozenMetav1Now, 123),
sadTransformationExpressionsCondition(here.Doc(
`spec.identityProvider[2].transforms.expressions[1].expression was invalid:
CEL expression compile error: ERROR: <input>:1:6: Syntax error: mismatched input 'does' expecting <EOF>
| this does not compile
| .....^
spec.identityProvider[2].transforms.expressions[3].expression was invalid:
CEL expression compile error: ERROR: <input>:1:6: Syntax error: mismatched input 'also' expecting <EOF>
| this also does not compile
| .....^`,
), frozenMetav1Now, 123),
sadTransformationExamplesCondition(here.Doc(
`.spec.identityProviders[0].transforms.examples[0] example failed:
expected: username "this is wrong string"
actual: username "ryan:suffix"
.spec.identityProviders[0].transforms.examples[0] example failed:
expected: groups ["this is wrong string list"]
actual: groups ["a", "b"]
.spec.identityProviders[0].transforms.examples[1] example failed:
expected: username "this is also wrong string"
actual: username "ryan:suffix"
.spec.identityProviders[0].transforms.examples[1] example failed:
expected: groups ["this is also wrong string list"]
actual: groups ["a", "b"]
.spec.identityProviders[1].transforms.examples[1] example failed:
expected: username "this is still wrong string"
actual: username "ryan:suffix"
.spec.identityProviders[1].transforms.examples[1] example failed:
expected: groups ["this is still wrong string list"]
actual: groups ["a", "b"]
unable to check if the examples specified by .spec.identityProviders[2].transforms.examples[] had errors because an expression was invalid`,
), frozenMetav1Now, 123),
sadReadyCondition(frozenMetav1Now, 123),
}),
),
expectedFederationDomainStatusUpdate(
&configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "config2", Namespace: namespace, Generation: 123},
},
configv1alpha1.FederationDomainPhaseError,
replaceConditions(
allHappyConditionsSuccess("https://not-unique.com", frozenMetav1Now, 123),
[]configv1alpha1.Condition{
sadIssuerIsUniqueCondition(frozenMetav1Now, 123),
sadTransformationExpressionsCondition(here.Doc(
`spec.identityProvider[0].transforms.expressions[1].expression was invalid:
CEL expression compile error: ERROR: <input>:1:6: Syntax error: mismatched input 'still' expecting <EOF>
| this still does not compile
| .....^
spec.identityProvider[0].transforms.expressions[3].expression was invalid:
CEL expression compile error: ERROR: <input>:1:6: Syntax error: mismatched input 'really' expecting <EOF>
| this really does not compile
| .....^`,
), frozenMetav1Now, 123),
sadTransformationExamplesCondition(
"unable to check if the examples specified by .spec.identityProviders[0].transforms.examples[] had errors because an expression was invalid",
frozenMetav1Now, 123), frozenMetav1Now, 123),
sadReadyCondition(frozenMetav1Now, 123), sadReadyCondition(frozenMetav1Now, 123),
}), }),