Validate transforms expressions in federation_domain_watcher.go

This commit is contained in:
Ryan Richard 2023-07-14 12:06:20 -07:00
parent 013030041a
commit 52925a2a46
3 changed files with 237 additions and 95 deletions

View File

@ -44,6 +44,7 @@ const (
typeIdentityProvidersAPIGroupSuffixValid = "IdentityProvidersObjectRefAPIGroupSuffixValid" typeIdentityProvidersAPIGroupSuffixValid = "IdentityProvidersObjectRefAPIGroupSuffixValid"
typeIdentityProvidersObjectRefKindValid = "IdentityProvidersObjectRefKindValid" typeIdentityProvidersObjectRefKindValid = "IdentityProvidersObjectRefKindValid"
typeTransformsConstantsNamesUnique = "TransformsConstantsNamesUnique" typeTransformsConstantsNamesUnique = "TransformsConstantsNamesUnique"
typeTransformsExpressionsValid = "TransformsExpressionsValid"
reasonSuccess = "Success" reasonSuccess = "Success"
reasonNotReady = "NotReady" reasonNotReady = "NotReady"
@ -59,6 +60,7 @@ const (
reasonAPIGroupNameUnrecognized = "APIGroupUnrecognized" reasonAPIGroupNameUnrecognized = "APIGroupUnrecognized"
reasonKindUnrecognized = "KindUnrecognized" reasonKindUnrecognized = "KindUnrecognized"
reasonDuplicateConstantsNames = "DuplicateConstantsNames" reasonDuplicateConstantsNames = "DuplicateConstantsNames"
reasonInvalidTransformsExpressions = "InvalidTransformsExpressions"
kindLDAPIdentityProvider = "LDAPIdentityProvider" kindLDAPIdentityProvider = "LDAPIdentityProvider"
kindOIDCIdentityProvider = "OIDCIdentityProvider" kindOIDCIdentityProvider = "OIDCIdentityProvider"
@ -162,7 +164,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
} }
// Process each FederationDomain to validate its spec and to turn it into a FederationDomainIssuer. // Process each FederationDomain to validate its spec and to turn it into a FederationDomainIssuer.
federationDomainIssuers, fdToConditionsMap, err := c.processAllFederationDomains(federationDomains) federationDomainIssuers, fdToConditionsMap, err := c.processAllFederationDomains(ctx.Context, federationDomains)
if err != nil { if err != nil {
return err return err
} }
@ -185,6 +187,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
} }
func (c *federationDomainWatcherController) processAllFederationDomains( func (c *federationDomainWatcherController) processAllFederationDomains(
ctx context.Context,
federationDomains []*configv1alpha1.FederationDomain, federationDomains []*configv1alpha1.FederationDomain,
) ([]*federationdomainproviders.FederationDomainIssuer, map[*configv1alpha1.FederationDomain][]*configv1alpha1.Condition, error) { ) ([]*federationdomainproviders.FederationDomainIssuer, map[*configv1alpha1.FederationDomain][]*configv1alpha1.Condition, error) {
federationDomainIssuers := make([]*federationdomainproviders.FederationDomainIssuer, 0) federationDomainIssuers := make([]*federationdomainproviders.FederationDomainIssuer, 0)
@ -196,7 +199,7 @@ func (c *federationDomainWatcherController) processAllFederationDomains(
conditions = crossDomainConfigValidator.Validate(federationDomain, conditions) conditions = crossDomainConfigValidator.Validate(federationDomain, conditions)
federationDomainIssuer, conditions, err := c.makeFederationDomainIssuer(federationDomain, conditions) federationDomainIssuer, conditions, err := c.makeFederationDomainIssuer(ctx, federationDomain, conditions)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -216,6 +219,7 @@ func (c *federationDomainWatcherController) processAllFederationDomains(
} }
func (c *federationDomainWatcherController) makeFederationDomainIssuer( func (c *federationDomainWatcherController) makeFederationDomainIssuer(
ctx context.Context,
federationDomain *configv1alpha1.FederationDomain, federationDomain *configv1alpha1.FederationDomain,
conditions []*configv1alpha1.Condition, conditions []*configv1alpha1.Condition,
) (*federationdomainproviders.FederationDomainIssuer, []*configv1alpha1.Condition, error) { ) (*federationdomainproviders.FederationDomainIssuer, []*configv1alpha1.Condition, error) {
@ -230,7 +234,7 @@ func (c *federationDomainWatcherController) makeFederationDomainIssuer(
return nil, nil, err return nil, nil, err
} }
} else { } else {
federationDomainIssuer, conditions, err = c.makeFederationDomainIssuerWithExplicitIDPs(federationDomain, conditions) federationDomainIssuer, conditions, err = c.makeFederationDomainIssuerWithExplicitIDPs(ctx, federationDomain, conditions)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
@ -313,19 +317,23 @@ func (c *federationDomainWatcherController) makeLegacyFederationDomainIssuer(
}) })
} }
// This is the constructor for the backwards compatibility mode. // This is the constructor for the legacy backwards compatibility mode.
federationDomainIssuer, err := federationdomainproviders.NewFederationDomainIssuerWithDefaultIDP(federationDomain.Spec.Issuer, defaultFederationDomainIdentityProvider) federationDomainIssuer, err := federationdomainproviders.NewFederationDomainIssuerWithDefaultIDP(federationDomain.Spec.Issuer, defaultFederationDomainIdentityProvider)
conditions = appendIssuerURLValidCondition(err, conditions) conditions = appendIssuerURLValidCondition(err, conditions)
// These conditions can only have errors when the list of IDPs is explicitly configured,
// and in this case there are no IDPs explicitly configured, so set these conditions all to have no errors.
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(sets.Set[string]{}, conditions)
conditions = appendTransformsExpressionsValidCondition([]string{}, conditions)
return federationDomainIssuer, conditions, nil return federationDomainIssuer, conditions, nil
} }
func (c *federationDomainWatcherController) makeFederationDomainIssuerWithExplicitIDPs( func (c *federationDomainWatcherController) makeFederationDomainIssuerWithExplicitIDPs(
ctx context.Context,
federationDomain *configv1alpha1.FederationDomain, federationDomain *configv1alpha1.FederationDomain,
conditions []*configv1alpha1.Condition, conditions []*configv1alpha1.Condition,
) (*federationdomainproviders.FederationDomainIssuer, []*configv1alpha1.Condition, error) { ) (*federationdomainproviders.FederationDomainIssuer, []*configv1alpha1.Condition, error) {
@ -337,10 +345,13 @@ func (c *federationDomainWatcherController) makeFederationDomainIssuerWithExplic
badKinds := []string{} badKinds := []string{}
for index, idp := range federationDomain.Spec.IdentityProviders { for index, idp := range federationDomain.Spec.IdentityProviders {
idpIsValid := true
// The CRD requires the displayName field, and validates that it has at least one character, // The CRD requires the displayName field, and validates that it has at least one character,
// so here we only need to validate that they are unique. // so here we only need to validate that they are unique.
if displayNames.Has(idp.DisplayName) { if displayNames.Has(idp.DisplayName) {
duplicateDisplayNames.Insert(idp.DisplayName) duplicateDisplayNames.Insert(idp.DisplayName)
idpIsValid = false
} }
displayNames.Insert(idp.DisplayName) displayNames.Insert(idp.DisplayName)
@ -361,6 +372,7 @@ func (c *federationDomainWatcherController) makeFederationDomainIssuerWithExplic
canTryToFindIDP = false canTryToFindIDP = false
} }
// When the apiGroup and kind are valid, try to find the IDP CR.
var idpResourceUID types.UID var idpResourceUID types.UID
idpWasFound := false idpWasFound := false
if canTryToFindIDP { if canTryToFindIDP {
@ -375,26 +387,31 @@ func (c *federationDomainWatcherController) makeFederationDomainIssuerWithExplic
} }
if !canTryToFindIDP || !idpWasFound { if !canTryToFindIDP || !idpWasFound {
idpNotFoundIndices = append(idpNotFoundIndices, index) idpNotFoundIndices = append(idpNotFoundIndices, index)
idpIsValid = false
} }
var err error var err error
var pipeline *idtransform.TransformationPipeline var pipeline *idtransform.TransformationPipeline
pipeline, conditions, err = c.makeTransformationPipelineForIdentityProvider(idp, federationDomain.Name, conditions) var allExamplesPassed bool
pipeline, allExamplesPassed, conditions, err = c.makeTransformationPipelineAndEvaluateExamplesForIdentityProvider(ctx, idp, index, conditions)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
} }
if !allExamplesPassed {
idpIsValid = false
}
// For each valid IDP (unique displayName, valid objectRef + valid transforms), add it to the list. if !idpIsValid {
// Something about the IDP was not valid. Don't add it.
continue
}
// For a valid IDP (unique displayName, valid objectRef, valid transforms), add it to the list.
federationDomainIdentityProviders = append(federationDomainIdentityProviders, &federationdomainproviders.FederationDomainIdentityProvider{ federationDomainIdentityProviders = append(federationDomainIdentityProviders, &federationdomainproviders.FederationDomainIdentityProvider{
DisplayName: idp.DisplayName, DisplayName: idp.DisplayName,
UID: idpResourceUID, UID: idpResourceUID,
Transforms: pipeline, Transforms: pipeline,
}) })
plog.Debug("loaded FederationDomain identity provider",
"federationDomain", federationDomain.Name,
"identityProviderDisplayName", idp.DisplayName,
"identityProviderResourceUID", idpResourceUID,
)
} }
if len(idpNotFoundIndices) != 0 { if len(idpNotFoundIndices) != 0 {
@ -457,12 +474,31 @@ func (c *federationDomainWatcherController) findIDPsUIDByObjectRef(objectRef cor
return idpResourceUID, true, nil return idpResourceUID, true, nil
} }
func (c *federationDomainWatcherController) makeTransformationPipelineForIdentityProvider( func (c *federationDomainWatcherController) makeTransformationPipelineAndEvaluateExamplesForIdentityProvider(
ctx context.Context,
idp configv1alpha1.FederationDomainIdentityProvider, idp configv1alpha1.FederationDomainIdentityProvider,
federationDomainName string, idpIndex int,
conditions []*configv1alpha1.Condition, conditions []*configv1alpha1.Condition,
) (*idtransform.TransformationPipeline, []*configv1alpha1.Condition, error) { ) (*idtransform.TransformationPipeline, bool, []*configv1alpha1.Condition, error) {
pipeline := idtransform.NewTransformationPipeline() consts, conditions, err := c.makeTransformsConstants(idp, conditions)
if err != nil {
return nil, false, nil, err
}
pipeline, conditions, err := c.makeTransformationPipeline(idp, idpIndex, consts, conditions)
if err != nil {
return nil, false, nil, err
}
allExamplesPassed, conditions := c.evaluateExamples(ctx, idp, pipeline, conditions)
return pipeline, allExamplesPassed, conditions, nil
}
func (c *federationDomainWatcherController) makeTransformsConstants(
idp configv1alpha1.FederationDomainIdentityProvider,
conditions []*configv1alpha1.Condition,
) (*celtransformer.TransformationConstants, []*configv1alpha1.Condition, error) {
consts := &celtransformer.TransformationConstants{ consts := &celtransformer.TransformationConstants{
StringConstants: map[string]string{}, StringConstants: map[string]string{},
StringListConstants: map[string][]string{}, StringListConstants: map[string][]string{},
@ -488,10 +524,23 @@ func (c *federationDomainWatcherController) makeTransformationPipelineForIdentit
return nil, nil, fmt.Errorf("one of spec.identityProvider[].transforms.constants[].type is invalid: %q", constant.Type) return nil, nil, fmt.Errorf("one of spec.identityProvider[].transforms.constants[].type is invalid: %q", constant.Type)
} }
} }
conditions = appendTransformsConstantsNamesUniqueCondition(duplicateConstNames, conditions) conditions = appendTransformsConstantsNamesUniqueCondition(duplicateConstNames, conditions)
return consts, conditions, nil
}
func (c *federationDomainWatcherController) makeTransformationPipeline(
idp configv1alpha1.FederationDomainIdentityProvider,
idpIndex int,
consts *celtransformer.TransformationConstants,
conditions []*configv1alpha1.Condition,
) (*idtransform.TransformationPipeline, []*configv1alpha1.Condition, error) {
pipeline := idtransform.NewTransformationPipeline()
expressionsCompileErrors := []string{}
// Compile all the expressions and add them to the pipeline. // Compile all the expressions and add them to the pipeline.
for idx, expr := range idp.Transforms.Expressions { for exprIndex, expr := range idp.Transforms.Expressions {
var rawTransform celtransformer.CELTransformation var rawTransform celtransformer.CELTransformation
switch expr.Type { switch expr.Type {
case "username/v1": case "username/v1":
@ -507,37 +556,50 @@ func (c *federationDomainWatcherController) makeTransformationPipelineForIdentit
// 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, 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)
if err != nil { if err != nil {
// TODO: handle compile err expressionsCompileErrors = append(expressionsCompileErrors,
plog.Error("error compiling identity transformation", err, fmt.Sprintf("spec.identityProvider[%d].transforms.expressions[%d].expression was invalid:\n%s",
"federationDomain", federationDomainName, idpIndex, exprIndex, err.Error()))
"idpDisplayName", idp.DisplayName,
"transformationIndex", idx,
"transformationType", expr.Type,
"transformationExpression", expr.Expression,
)
} }
pipeline.AppendTransformation(compiledTransform) pipeline.AppendTransformation(compiledTransform)
plog.Debug("successfully compiled identity transformation expression", }
"type", expr.Type,
"expr", expr.Expression, conditions = appendTransformsExpressionsValidCondition(expressionsCompileErrors, conditions)
"policyMessage", expr.Message,
) if len(expressionsCompileErrors) > 0 {
// One or more of the expressions did not compile, so we don't have a useful pipeline to return.
return nil, conditions, nil
}
return pipeline, conditions, nil
}
func (c *federationDomainWatcherController) evaluateExamples(
ctx context.Context,
idp configv1alpha1.FederationDomainIdentityProvider,
pipeline *idtransform.TransformationPipeline,
conditions []*configv1alpha1.Condition,
) (bool, []*configv1alpha1.Condition) {
if pipeline == nil {
// TODO cannot evaluate examples, but still need to write a condition for it
return false, conditions
} }
// 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.
for idx, e := range idp.Transforms.Examples { examplesErrors := []string{}
// TODO: use a real context param below for examplesIndex, e := range idp.Transforms.Examples {
result, _ := pipeline.Evaluate(context.TODO(), e.Username, e.Groups) result, _ := pipeline.Evaluate(ctx, e.Username, e.Groups)
// TODO: handle err // TODO: handle err
resultWasAuthRejected := !result.AuthenticationAllowed resultWasAuthRejected := !result.AuthenticationAllowed
if e.Expects.Rejected && !resultWasAuthRejected { //nolint:gocritic,nestif if e.Expects.Rejected && !resultWasAuthRejected { //nolint:gocritic,nestif
// TODO: handle this failed example // TODO: handle this failed example
examplesErrors = append(examplesErrors, "TODO")
plog.Warning("FederationDomain identity provider transformations example failed: expected authentication to be rejected but it was not", plog.Warning("FederationDomain identity provider transformations example failed: expected authentication to be rejected but it was not",
"federationDomain", federationDomainName,
"idpDisplayName", idp.DisplayName, "idpDisplayName", idp.DisplayName,
"exampleIndex", idx, "exampleIndex", examplesIndex,
"expectedRejected", e.Expects.Rejected, "expectedRejected", e.Expects.Rejected,
"actualRejectedResult", resultWasAuthRejected, "actualRejectedResult", resultWasAuthRejected,
"expectedMessage", e.Expects.Message, "expectedMessage", e.Expects.Message,
@ -545,10 +607,10 @@ func (c *federationDomainWatcherController) makeTransformationPipelineForIdentit
) )
} else if !e.Expects.Rejected && resultWasAuthRejected { } else if !e.Expects.Rejected && resultWasAuthRejected {
// TODO: handle this failed example // TODO: handle this failed example
examplesErrors = append(examplesErrors, "TODO")
plog.Warning("FederationDomain identity provider transformations example failed: expected authentication not to be rejected but it was rejected", plog.Warning("FederationDomain identity provider transformations example failed: expected authentication not to be rejected but it was rejected",
"federationDomain", federationDomainName,
"idpDisplayName", idp.DisplayName, "idpDisplayName", idp.DisplayName,
"exampleIndex", idx, "exampleIndex", examplesIndex,
"expectedRejected", e.Expects.Rejected, "expectedRejected", e.Expects.Rejected,
"actualRejectedResult", resultWasAuthRejected, "actualRejectedResult", resultWasAuthRejected,
"expectedMessage", e.Expects.Message, "expectedMessage", e.Expects.Message,
@ -557,10 +619,10 @@ func (c *federationDomainWatcherController) makeTransformationPipelineForIdentit
} else if e.Expects.Rejected && resultWasAuthRejected && e.Expects.Message != result.RejectedAuthenticationMessage { } else if e.Expects.Rejected && resultWasAuthRejected && e.Expects.Message != result.RejectedAuthenticationMessage {
// TODO: when expected message is blank, then treat it like it expects the default message // TODO: when expected message is blank, then treat it like it expects the default message
// TODO: handle this failed example // TODO: handle this failed example
examplesErrors = append(examplesErrors, "TODO")
plog.Warning("FederationDomain identity provider transformations example failed: expected a different authentication rejection message", plog.Warning("FederationDomain identity provider transformations example failed: expected a different authentication rejection message",
"federationDomain", federationDomainName,
"idpDisplayName", idp.DisplayName, "idpDisplayName", idp.DisplayName,
"exampleIndex", idx, "exampleIndex", examplesIndex,
"expectedRejected", e.Expects.Rejected, "expectedRejected", e.Expects.Rejected,
"actualRejectedResult", resultWasAuthRejected, "actualRejectedResult", resultWasAuthRejected,
"expectedMessage", e.Expects.Message, "expectedMessage", e.Expects.Message,
@ -572,10 +634,10 @@ func (c *federationDomainWatcherController) makeTransformationPipelineForIdentit
// TODO: when both of these fail, put both errors onto the status (not just the first one) // TODO: when both of these fail, put both errors onto the status (not just the first one)
if e.Expects.Username != result.Username { if e.Expects.Username != result.Username {
// TODO: handle this failed example // TODO: handle this failed example
examplesErrors = append(examplesErrors, "TODO")
plog.Warning("FederationDomain identity provider transformations example failed: expected a different transformed username", plog.Warning("FederationDomain identity provider transformations example failed: expected a different transformed username",
"federationDomain", federationDomainName,
"idpDisplayName", idp.DisplayName, "idpDisplayName", idp.DisplayName,
"exampleIndex", idx, "exampleIndex", examplesIndex,
"expectedUsername", e.Expects.Username, "expectedUsername", e.Expects.Username,
"actualUsernameResult", result.Username, "actualUsernameResult", result.Username,
) )
@ -584,10 +646,10 @@ func (c *federationDomainWatcherController) makeTransformationPipelineForIdentit
// TODO: Do we need to make this insensitive to ordering, or should the transformations evaluator be changed to always return sorted group names at the end of the pipeline? // TODO: Do we need to make this insensitive to ordering, or should the transformations evaluator be changed to always return sorted group names at the end of the pipeline?
// TODO: What happens if the user did not write any group expectation? Treat it like expecting an empty list of groups? // TODO: What happens if the user did not write any group expectation? Treat it like expecting an empty list of groups?
// TODO: handle this failed example // TODO: handle this failed example
examplesErrors = append(examplesErrors, "TODO")
plog.Warning("FederationDomain identity provider transformations example failed: expected a different transformed groups list", plog.Warning("FederationDomain identity provider transformations example failed: expected a different transformed groups list",
"federationDomain", federationDomainName,
"idpDisplayName", idp.DisplayName, "idpDisplayName", idp.DisplayName,
"exampleIndex", idx, "exampleIndex", examplesIndex,
"expectedGroups", e.Expects.Groups, "expectedGroups", e.Expects.Groups,
"actualGroupsResult", result.Groups, "actualGroupsResult", result.Groups,
) )
@ -595,7 +657,7 @@ func (c *federationDomainWatcherController) makeTransformationPipelineForIdentit
} }
} }
return pipeline, conditions, nil return len(examplesErrors) == 0, conditions
} }
func (c *federationDomainWatcherController) sortedAllowedKinds() []string { func (c *federationDomainWatcherController) sortedAllowedKinds() []string {
@ -642,6 +704,26 @@ func appendIdentityProviderObjectRefAPIGroupSuffixCondition(expectedSuffixName s
return conditions return conditions
} }
func appendTransformsExpressionsValidCondition(errors []string, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
if len(errors) > 0 {
conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsExpressionsValid,
Status: configv1alpha1.ConditionFalse,
Reason: reasonInvalidTransformsExpressions,
Message: fmt.Sprintf("the expressions specified by .spec.identityProviders[].transforms.expressions[] were invalid:\n\n%s",
strings.Join(errors, "\n\n")),
})
} else {
conditions = append(conditions, &configv1alpha1.Condition{
Type: typeTransformsExpressionsValid,
Status: configv1alpha1.ConditionTrue,
Reason: reasonSuccess,
Message: "the expressions specified by .spec.identityProviders[].transforms.expressions[] are valid",
})
}
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{

View File

@ -27,6 +27,7 @@ import (
pinnipedinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions" pinnipedinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions"
"go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/controllerlib"
"go.pinniped.dev/internal/federationdomain/federationdomainproviders" "go.pinniped.dev/internal/federationdomain/federationdomainproviders"
"go.pinniped.dev/internal/here"
"go.pinniped.dev/internal/idtransform" "go.pinniped.dev/internal/idtransform"
"go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil"
) )
@ -419,6 +420,28 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
} }
} }
happyTransformationExpressionsCondition := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "TransformsExpressionsValid",
Status: "True",
ObservedGeneration: observedGeneration,
LastTransitionTime: time,
Reason: "Success",
Message: "the expressions specified by .spec.identityProviders[].transforms.expressions[] are valid",
}
}
sadTransformationExpressionsCondition := func(errorMessages string, time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
return configv1alpha1.Condition{
Type: "TransformsExpressionsValid",
Status: "False",
ObservedGeneration: observedGeneration,
LastTransitionTime: time,
Reason: "InvalidTransformsExpressions",
Message: fmt.Sprintf("the expressions specified by .spec.identityProviders[].transforms.expressions[] were invalid:\n\n%s", errorMessages),
}
}
happyAPIGroupSuffixCondition := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition { happyAPIGroupSuffixCondition := func(time metav1.Time, observedGeneration int64) configv1alpha1.Condition {
return configv1alpha1.Condition{ return configv1alpha1.Condition{
Type: "IdentityProvidersObjectRefAPIGroupSuffixValid", Type: "IdentityProvidersObjectRefAPIGroupSuffixValid",
@ -474,22 +497,21 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
return cp return cp
} }
allHappyConditionsLegacyConfigurationSuccess := func(issuer string, idpName string, time metav1.Time, observedGeneration int64) []configv1alpha1.Condition { replaceConditions := func(conditions []configv1alpha1.Condition, sadConditions []configv1alpha1.Condition) []configv1alpha1.Condition {
return sortConditionsByType([]configv1alpha1.Condition{ for _, sadReplaceCondition := range sadConditions {
happyConstNamesUniqueCondition(frozenMetav1Now, 123), for origIndex, origCondition := range conditions {
happyKindCondition(frozenMetav1Now, 123), if origCondition.Type == sadReplaceCondition.Type {
happyAPIGroupSuffixCondition(frozenMetav1Now, 123), conditions[origIndex] = sadReplaceCondition
happyDisplayNamesUniqueCondition(frozenMetav1Now, 123), break
happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(idpName, time, observedGeneration), }
happyIssuerIsUniqueCondition(time, observedGeneration), }
happyIssuerURLValidCondition(time, observedGeneration), }
happyOneTLSSecretPerIssuerHostnameCondition(time, observedGeneration), return conditions
happyReadyCondition(issuer, time, observedGeneration),
})
} }
allHappyConditionsSuccess := func(issuer string, time metav1.Time, observedGeneration int64) []configv1alpha1.Condition { allHappyConditionsSuccess := func(issuer string, time metav1.Time, observedGeneration int64) []configv1alpha1.Condition {
return sortConditionsByType([]configv1alpha1.Condition{ return sortConditionsByType([]configv1alpha1.Condition{
happyTransformationExpressionsCondition(frozenMetav1Now, 123),
happyConstNamesUniqueCondition(frozenMetav1Now, 123), happyConstNamesUniqueCondition(frozenMetav1Now, 123),
happyKindCondition(frozenMetav1Now, 123), happyKindCondition(frozenMetav1Now, 123),
happyAPIGroupSuffixCondition(frozenMetav1Now, 123), happyAPIGroupSuffixCondition(frozenMetav1Now, 123),
@ -502,16 +524,13 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
}) })
} }
replaceConditions := func(conditions []configv1alpha1.Condition, sadConditions []configv1alpha1.Condition) []configv1alpha1.Condition { allHappyConditionsLegacyConfigurationSuccess := func(issuer string, idpName string, time metav1.Time, observedGeneration int64) []configv1alpha1.Condition {
for _, sadReplaceCondition := range sadConditions { return replaceConditions(
for origIndex, origCondition := range conditions { allHappyConditionsSuccess(issuer, time, observedGeneration),
if origCondition.Type == sadReplaceCondition.Type { []configv1alpha1.Condition{
conditions[origIndex] = sadReplaceCondition happyIdentityProvidersFoundConditionLegacyConfigurationSuccess(idpName, time, observedGeneration),
break },
} )
}
}
return conditions
} }
invalidIssuerURL := ":/host//path" invalidIssuerURL := ":/host//path"
@ -1228,36 +1247,12 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
}, },
Transforms: configv1alpha1.FederationDomainTransforms{ Transforms: configv1alpha1.FederationDomainTransforms{
Constants: []configv1alpha1.FederationDomainTransformsConstant{ Constants: []configv1alpha1.FederationDomainTransformsConstant{
{ {Name: "duplicate1", Type: "string", StringValue: "abc"},
Name: "duplicate1", {Name: "duplicate1", Type: "string", StringValue: "def"},
Type: "string", {Name: "duplicate1", Type: "string", StringValue: "efg"},
StringValue: "abc", {Name: "duplicate2", Type: "string", StringValue: "123"},
}, {Name: "duplicate2", Type: "string", StringValue: "456"},
{ {Name: "uniqueName", Type: "string", StringValue: "hij"},
Name: "duplicate1",
Type: "string",
StringValue: "def",
},
{
Name: "duplicate1",
Type: "string",
StringValue: "efg",
},
{
Name: "duplicate2",
Type: "string",
StringValue: "123",
},
{
Name: "duplicate2",
Type: "string",
StringValue: "456",
},
{
Name: "unique",
Type: "string",
StringValue: "hij",
},
}, },
}, },
}, },
@ -1281,6 +1276,67 @@ func TestTestFederationDomainWatcherControllerSync(t *testing.T) {
), ),
}, },
}, },
{
name: "the federation domain has transformation expressions which don't compile",
inputObjects: []runtime.Object{
oidcIdentityProvider,
&configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
Spec: configv1alpha1.FederationDomainSpec{
Issuer: "https://issuer1.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: "this is not a valid cel expression"},
{Type: "groups/v1", Expression: "this is also not a valid cel expression"},
{Type: "username/v1", Expression: "username"}, // valid
{Type: "policy/v1", Expression: "still not a valid cel expression"},
},
},
},
},
},
},
},
wantFDIssuers: []*federationdomainproviders.FederationDomainIssuer{},
wantStatusUpdates: []*configv1alpha1.FederationDomain{
expectedFederationDomainStatusUpdate(
&configv1alpha1.FederationDomain{
ObjectMeta: metav1.ObjectMeta{Name: "config1", Namespace: namespace, Generation: 123},
},
configv1alpha1.FederationDomainPhaseError,
replaceConditions(
allHappyConditionsSuccess("https://issuer1.com", frozenMetav1Now, 123),
[]configv1alpha1.Condition{
sadTransformationExpressionsCondition(
here.Doc(
`spec.identityProvider[0].transforms.expressions[0].expression was invalid:
CEL expression compile error: ERROR: <input>:1:6: Syntax error: mismatched input 'is' expecting <EOF>
| this is not a valid cel expression
| .....^
spec.identityProvider[0].transforms.expressions[1].expression was invalid:
CEL expression compile error: ERROR: <input>:1:6: Syntax error: mismatched input 'is' expecting <EOF>
| this is also not a valid cel expression
| .....^
spec.identityProvider[0].transforms.expressions[3].expression was invalid:
CEL expression compile error: ERROR: <input>:1:7: Syntax error: mismatched input 'not' expecting <EOF>
| still not a valid cel expression
| ......^`),
frozenMetav1Now, 123),
sadReadyCondition(frozenMetav1Now, 123),
}),
),
},
},
{ {
name: "the federation domain specifies illegal const type, which shouldn't really happen since the CRD validates it", name: "the federation domain specifies illegal const type, which shouldn't really happen since the CRD validates it",
inputObjects: []runtime.Object{ inputObjects: []runtime.Object{

View File

@ -142,6 +142,7 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) {
"IdentityProvidersObjectRefAPIGroupSuffixValid": v1alpha1.ConditionTrue, "IdentityProvidersObjectRefAPIGroupSuffixValid": v1alpha1.ConditionTrue,
"IdentityProvidersDisplayNamesUnique": v1alpha1.ConditionTrue, "IdentityProvidersDisplayNamesUnique": v1alpha1.ConditionTrue,
"TransformsConstantsNamesUnique": v1alpha1.ConditionTrue, "TransformsConstantsNamesUnique": v1alpha1.ConditionTrue,
"TransformsExpressionsValid": v1alpha1.ConditionTrue,
}) })
requireStatus(t, client, ns, config6Duplicate2.Name, v1alpha1.FederationDomainPhaseError, map[string]v1alpha1.ConditionStatus{ requireStatus(t, client, ns, config6Duplicate2.Name, v1alpha1.FederationDomainPhaseError, map[string]v1alpha1.ConditionStatus{
"Ready": v1alpha1.ConditionFalse, "Ready": v1alpha1.ConditionFalse,
@ -153,6 +154,7 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) {
"IdentityProvidersObjectRefAPIGroupSuffixValid": v1alpha1.ConditionTrue, "IdentityProvidersObjectRefAPIGroupSuffixValid": v1alpha1.ConditionTrue,
"IdentityProvidersDisplayNamesUnique": v1alpha1.ConditionTrue, "IdentityProvidersDisplayNamesUnique": v1alpha1.ConditionTrue,
"TransformsConstantsNamesUnique": v1alpha1.ConditionTrue, "TransformsConstantsNamesUnique": v1alpha1.ConditionTrue,
"TransformsExpressionsValid": v1alpha1.ConditionTrue,
}) })
requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, issuer6) requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, issuer6)
@ -181,6 +183,7 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) {
"IdentityProvidersObjectRefAPIGroupSuffixValid": v1alpha1.ConditionTrue, "IdentityProvidersObjectRefAPIGroupSuffixValid": v1alpha1.ConditionTrue,
"IdentityProvidersDisplayNamesUnique": v1alpha1.ConditionTrue, "IdentityProvidersDisplayNamesUnique": v1alpha1.ConditionTrue,
"TransformsConstantsNamesUnique": v1alpha1.ConditionTrue, "TransformsConstantsNamesUnique": v1alpha1.ConditionTrue,
"TransformsExpressionsValid": v1alpha1.ConditionTrue,
}) })
requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, badIssuer) requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, badIssuer)
requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, badConfig, client, ns, scheme, addr, caBundle, badIssuer) requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, badConfig, client, ns, scheme, addr, caBundle, badIssuer)
@ -698,6 +701,7 @@ func requireFullySuccessfulStatus(t *testing.T, client pinnipedclientset.Interfa
"IdentityProvidersObjectRefAPIGroupSuffixValid": v1alpha1.ConditionTrue, "IdentityProvidersObjectRefAPIGroupSuffixValid": v1alpha1.ConditionTrue,
"IdentityProvidersDisplayNamesUnique": v1alpha1.ConditionTrue, "IdentityProvidersDisplayNamesUnique": v1alpha1.ConditionTrue,
"TransformsConstantsNamesUnique": v1alpha1.ConditionTrue, "TransformsConstantsNamesUnique": v1alpha1.ConditionTrue,
"TransformsExpressionsValid": v1alpha1.ConditionTrue,
}) })
} }