Refactor: extract helper functions in federation_domain_watcher.go

Co-authored-by: Benjamin A. Petersen <ben@benjaminapetersen.me>
This commit is contained in:
Ryan Richard 2023-07-11 13:25:08 -07:00
parent a38fb16295
commit 76709892bc

View File

@ -10,6 +10,7 @@ import (
"strings"
"time"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -68,6 +69,8 @@ type federationDomainWatcherController struct {
oidcIdentityProviderInformer idpinformers.OIDCIdentityProviderInformer
ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer
activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer
celTransformer *celtransformer.CELTransformer
}
// NewFederationDomainWatcherController creates a controllerlib.Controller that watches
@ -125,33 +128,92 @@ func NewFederationDomainWatcherController(
}
// Sync implements controllerlib.Syncer.
func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) error { //nolint:funlen,gocyclo
func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) error {
federationDomains, err := c.federationDomainInformer.Lister().List(labels.Everything())
if err != nil {
return err
}
if c.celTransformer == nil {
c.celTransformer, _ = celtransformer.NewCELTransformer(celTransformerMaxExpressionRuntime) // TODO: what is a good duration limit here?
// TODO: handle err from NewCELTransformer() above
}
// Process each FederationDomain to validate its spec and to turn it into a FederationDomainIssuer.
federationDomainIssuers, fdToConditionsMap, _ := c.processAllFederationDomains(federationDomains)
// TODO: handle err
// Load the endpoints of every valid FederationDomain. Removes the endpoints of any
// previous FederationDomains which no longer exist or are no longer valid.
c.federationDomainsSetter.SetFederationDomains(federationDomainIssuers...)
// Now that the endpoints of every valid FederationDomain are available, update the
// statuses. This allows clients to wait for Ready without any race conditions in the
// endpoints being available.
var errs []error
for federationDomain, conditions := range fdToConditionsMap {
if err = c.updateStatus(ctx.Context, federationDomain, conditions); err != nil {
errs = append(errs, fmt.Errorf("could not update status: %w", err))
}
}
return errorsutil.NewAggregate(errs)
}
func (c *federationDomainWatcherController) processAllFederationDomains(
federationDomains []*configv1alpha1.FederationDomain,
) ([]*federationdomainproviders.FederationDomainIssuer, map[*configv1alpha1.FederationDomain][]*configv1alpha1.Condition, error) {
federationDomainIssuers := make([]*federationdomainproviders.FederationDomainIssuer, 0)
crossDomainConfigValidator := newCrossFederationDomainConfigValidator(federationDomains)
fdToConditionsMap := map[*configv1alpha1.FederationDomain][]*configv1alpha1.Condition{}
crossDomainConfigValidator := newCrossFederationDomainConfigValidator(federationDomains)
for _, federationDomain := range federationDomains {
conditions := make([]*configv1alpha1.Condition, 0, 4)
conditions := make([]*configv1alpha1.Condition, 0)
conditions = crossDomainConfigValidator.Validate(federationDomain, conditions)
// TODO: Move all this identity provider stuff into helper functions. This is just a sketch of how the code would
// work in the sense that this is not doing error handling, is not validating everything that it should, and
// is not updating the status of the FederationDomain with anything related to these identity providers.
// This code may crash on invalid inputs since it is not handling any errors. However, when given valid inputs,
// this correctly implements the multiple IDPs features.
federationDomainIssuer, conditions, _ := c.makeFederationDomainIssuer(federationDomain, conditions)
// TODO: handle err
// Now that we have determined the conditions, save them for after the loop.
// For a valid FederationDomain, want to update the conditions after we have
// made the FederationDomain's endpoints available.
fdToConditionsMap[federationDomain] = conditions
if !hadErrorCondition(conditions) {
// Successfully validated the FederationDomain, so allow it to be loaded.
federationDomainIssuers = append(federationDomainIssuers, federationDomainIssuer)
}
}
return federationDomainIssuers, fdToConditionsMap, nil
}
func (c *federationDomainWatcherController) makeFederationDomainIssuer(
federationDomain *configv1alpha1.FederationDomain,
conditions []*configv1alpha1.Condition,
) (*federationdomainproviders.FederationDomainIssuer, []*configv1alpha1.Condition, error) {
// Create the list of IDPs for this FederationDomain.
// Don't worry if the IDP CRs themselves is phase=Ready because those which are not ready will not be loaded
// into the provider cache, so they cannot actually be used to authenticate.
federationDomainIdentityProviders := []*federationdomainproviders.FederationDomainIdentityProvider{}
var defaultFederationDomainIdentityProvider *federationdomainproviders.FederationDomainIdentityProvider
var federationDomainIssuer *federationdomainproviders.FederationDomainIssuer
if len(federationDomain.Spec.IdentityProviders) == 0 {
federationDomainIssuer, conditions, _ = c.makeLegacyFederationDomainIssuer(federationDomain, conditions)
// TODO handle err
} else {
federationDomainIssuer, conditions, _ = c.makeFederationDomainIssuerWithExplicitIDPs(federationDomain, conditions)
// TODO handle err
}
return federationDomainIssuer, conditions, nil
}
func (c *federationDomainWatcherController) makeLegacyFederationDomainIssuer(
federationDomain *configv1alpha1.FederationDomain,
conditions []*configv1alpha1.Condition,
) (*federationdomainproviders.FederationDomainIssuer, []*configv1alpha1.Condition, error) {
var defaultFederationDomainIdentityProvider *federationdomainproviders.FederationDomainIdentityProvider
// When the FederationDomain does not list any IDPs, then we might be in backwards compatibility mode.
oidcIdentityProviders, _ := c.oidcIdentityProviderInformer.Lister().List(labels.Everything())
ldapIdentityProviders, _ := c.ldapIdentityProviderInformer.Lister().List(labels.Everything())
@ -211,159 +273,37 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
"resources have been found: please create an identity provider resource",
})
}
}
// If there is an explicit list of IDPs on the FederationDomain, then process the list.
celTransformer, _ := celtransformer.NewCELTransformer(celTransformerMaxExpressionRuntime) // TODO: what is a good duration limit here?
// TODO: handle err from NewCELTransformer() above
// This is the constructor for the backwards compatibility mode.
federationDomainIssuer, err := federationdomainproviders.NewFederationDomainIssuerWithDefaultIDP(federationDomain.Spec.Issuer, defaultFederationDomainIdentityProvider)
conditions = appendIssuerURLValidCondition(err, conditions)
return federationDomainIssuer, conditions, nil
}
func (c *federationDomainWatcherController) makeFederationDomainIssuerWithExplicitIDPs(
federationDomain *configv1alpha1.FederationDomain,
conditions []*configv1alpha1.Condition,
) (*federationdomainproviders.FederationDomainIssuer, []*configv1alpha1.Condition, error) {
var err error
federationDomainIdentityProviders := []*federationdomainproviders.FederationDomainIdentityProvider{}
idpNotFoundIndices := []int{}
for index, idp := range federationDomain.Spec.IdentityProviders {
// TODO: Validate that all displayNames are unique within this FederationDomain's spec's list of identity providers.
// TODO: Validate that idp.ObjectRef.APIGroup is the expected APIGroup for IDP CRs "idp.supervisor.pinniped.dev"
// Validate that each objectRef resolves to an existing IDP. It does not matter if the IDP itself
// is phase=Ready, because it will not be loaded into the cache if not ready. For each objectRef
// that does not resolve, put an error on the FederationDomain status.
var idpResourceUID types.UID
var foundIDP metav1.Object
switch idp.ObjectRef.Kind {
case "LDAPIdentityProvider":
foundIDP, err = c.ldapIdentityProviderInformer.Lister().LDAPIdentityProviders(federationDomain.Namespace).Get(idp.ObjectRef.Name)
case "ActiveDirectoryIdentityProvider":
foundIDP, err = c.activeDirectoryIdentityProviderInformer.Lister().ActiveDirectoryIdentityProviders(federationDomain.Namespace).Get(idp.ObjectRef.Name)
case "OIDCIdentityProvider":
foundIDP, err = c.oidcIdentityProviderInformer.Lister().OIDCIdentityProviders(federationDomain.Namespace).Get(idp.ObjectRef.Name)
default:
// TODO: handle an IDP type that we do not understand.
}
switch {
case err == nil:
idpResourceUID = foundIDP.GetUID()
case errors.IsNotFound(err):
idpResourceUID, idpWasFound, _ := c.findIDPsUIDByObjectRef(idp.ObjectRef, federationDomain.Namespace)
// TODO handle err
if !idpWasFound {
idpNotFoundIndices = append(idpNotFoundIndices, index)
default:
// TODO: handle unexpected errors
}
// Prepare the transformations.
pipeline := idtransform.NewTransformationPipeline()
consts := &celtransformer.TransformationConstants{
StringConstants: map[string]string{},
StringListConstants: map[string][]string{},
}
// Read all the declared constants.
for _, c := range idp.Transforms.Constants {
switch c.Type {
case "string":
consts.StringConstants[c.Name] = c.StringValue
case "stringList":
consts.StringListConstants[c.Name] = c.StringListValue
default:
// TODO: this shouldn't really happen since the CRD validates it, but handle it as an error
}
}
// Compile all the expressions and add them to the pipeline.
for idx, e := range idp.Transforms.Expressions {
var rawTransform celtransformer.CELTransformation
switch e.Type {
case "username/v1":
rawTransform = &celtransformer.UsernameTransformation{Expression: e.Expression}
case "groups/v1":
rawTransform = &celtransformer.GroupsTransformation{Expression: e.Expression}
case "policy/v1":
rawTransform = &celtransformer.AllowAuthenticationPolicy{
Expression: e.Expression,
RejectedAuthenticationMessage: e.Message,
}
default:
// TODO: this shouldn't really happen since the CRD validates it, but handle it as an error
}
compiledTransform, err := celTransformer.CompileTransformation(rawTransform, consts)
if err != nil {
// TODO: handle compile err
plog.Error("error compiling identity transformation", err,
"federationDomain", federationDomain.Name,
"idpDisplayName", idp.DisplayName,
"transformationIndex", idx,
"transformationType", e.Type,
"transformationExpression", e.Expression,
)
}
pipeline.AppendTransformation(compiledTransform)
plog.Debug("successfully compiled identity transformation expression",
"type", e.Type,
"expr", e.Expression,
"policyMessage", e.Message,
)
}
// Run all the provided transform examples. If any fail, put errors on the FederationDomain status.
for idx, e := range idp.Transforms.Examples {
// TODO: use a real context param below
result, _ := pipeline.Evaluate(context.TODO(), e.Username, e.Groups)
// TODO: handle err
resultWasAuthRejected := !result.AuthenticationAllowed
if e.Expects.Rejected && !resultWasAuthRejected { //nolint:gocritic,nestif
// TODO: handle this failed example
plog.Warning("FederationDomain identity provider transformations example failed: expected authentication to be rejected but it was not",
"federationDomain", federationDomain.Name,
"idpDisplayName", idp.DisplayName,
"exampleIndex", idx,
"expectedRejected", e.Expects.Rejected,
"actualRejectedResult", resultWasAuthRejected,
"expectedMessage", e.Expects.Message,
"actualMessageResult", result.RejectedAuthenticationMessage,
)
} else if !e.Expects.Rejected && resultWasAuthRejected {
// TODO: handle this failed example
plog.Warning("FederationDomain identity provider transformations example failed: expected authentication not to be rejected but it was rejected",
"federationDomain", federationDomain.Name,
"idpDisplayName", idp.DisplayName,
"exampleIndex", idx,
"expectedRejected", e.Expects.Rejected,
"actualRejectedResult", resultWasAuthRejected,
"expectedMessage", e.Expects.Message,
"actualMessageResult", 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: handle this failed example
plog.Warning("FederationDomain identity provider transformations example failed: expected a different authentication rejection message",
"federationDomain", federationDomain.Name,
"idpDisplayName", idp.DisplayName,
"exampleIndex", idx,
"expectedRejected", e.Expects.Rejected,
"actualRejectedResult", resultWasAuthRejected,
"expectedMessage", e.Expects.Message,
"actualMessageResult", result.RejectedAuthenticationMessage,
)
} else if result.AuthenticationAllowed {
// In the case where the user expected the auth to be allowed and it was allowed, then compare
// the expected username and group names to the actual username and group names.
// TODO: when both of these fail, put both errors onto the status (not just the first one)
if e.Expects.Username != result.Username {
// TODO: handle this failed example
plog.Warning("FederationDomain identity provider transformations example failed: expected a different transformed username",
"federationDomain", federationDomain.Name,
"idpDisplayName", idp.DisplayName,
"exampleIndex", idx,
"expectedUsername", e.Expects.Username,
"actualUsernameResult", result.Username,
)
}
if !stringSlicesEqual(e.Expects.Groups, result.Groups) {
// 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: handle this failed example
plog.Warning("FederationDomain identity provider transformations example failed: expected a different transformed groups list",
"federationDomain", federationDomain.Name,
"idpDisplayName", idp.DisplayName,
"exampleIndex", idx,
"expectedGroups", e.Expects.Groups,
"actualGroupsResult", result.Groups,
)
}
}
}
pipeline, _ := c.makeTransformationPipelineForIdentityProvider(idp, federationDomain.Name)
// TODO handle err
// For each valid IDP (unique displayName, valid objectRef + valid transforms), add it to the list.
federationDomainIdentityProviders = append(federationDomainIdentityProviders, &federationdomainproviders.FederationDomainIdentityProvider{
DisplayName: idp.DisplayName,
@ -399,15 +339,169 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
})
}
// Now that we have the list of IDPs for this FederationDomain, create the issuer.
var federationDomainIssuer *federationdomainproviders.FederationDomainIssuer
if defaultFederationDomainIdentityProvider != nil {
// This is the constructor for the backwards compatibility mode.
federationDomainIssuer, err = federationdomainproviders.NewFederationDomainIssuerWithDefaultIDP(federationDomain.Spec.Issuer, defaultFederationDomainIdentityProvider)
} else {
// This is the constructor for any other case, including when there is an empty list of IDPs.
federationDomainIssuer, err = federationdomainproviders.NewFederationDomainIssuer(federationDomain.Spec.Issuer, federationDomainIdentityProviders)
// This is the constructor for any case other than the legacy case, including when there is an empty list of IDPs.
federationDomainIssuer, err := federationdomainproviders.NewFederationDomainIssuer(federationDomain.Spec.Issuer, federationDomainIdentityProviders)
conditions = appendIssuerURLValidCondition(err, conditions)
return federationDomainIssuer, conditions, nil
}
func (c *federationDomainWatcherController) findIDPsUIDByObjectRef(objectRef corev1.TypedLocalObjectReference, namespace string) (types.UID, bool, error) {
var idpResourceUID types.UID
var foundIDP metav1.Object
var err error
switch objectRef.Kind {
case "LDAPIdentityProvider":
foundIDP, err = c.ldapIdentityProviderInformer.Lister().LDAPIdentityProviders(namespace).Get(objectRef.Name)
case "ActiveDirectoryIdentityProvider":
foundIDP, err = c.activeDirectoryIdentityProviderInformer.Lister().ActiveDirectoryIdentityProviders(namespace).Get(objectRef.Name)
case "OIDCIdentityProvider":
foundIDP, err = c.oidcIdentityProviderInformer.Lister().OIDCIdentityProviders(namespace).Get(objectRef.Name)
default:
// TODO: handle an IDP type that we do not understand.
}
switch {
case err == nil:
idpResourceUID = foundIDP.GetUID()
case errors.IsNotFound(err):
return "", false, nil
default:
// TODO: handle unexpected errors
}
return idpResourceUID, true, nil
}
func (c *federationDomainWatcherController) makeTransformationPipelineForIdentityProvider(
idp configv1alpha1.FederationDomainIdentityProvider,
federationDomainName string,
) (*idtransform.TransformationPipeline, error) {
pipeline := idtransform.NewTransformationPipeline()
consts := &celtransformer.TransformationConstants{
StringConstants: map[string]string{},
StringListConstants: map[string][]string{},
}
// Read all the declared constants.
for _, c := range idp.Transforms.Constants {
switch c.Type {
case "string":
consts.StringConstants[c.Name] = c.StringValue
case "stringList":
consts.StringListConstants[c.Name] = c.StringListValue
default:
// TODO: this shouldn't really happen since the CRD validates it, but handle it as an error
}
}
// Compile all the expressions and add them to the pipeline.
for idx, e := range idp.Transforms.Expressions {
var rawTransform celtransformer.CELTransformation
switch e.Type {
case "username/v1":
rawTransform = &celtransformer.UsernameTransformation{Expression: e.Expression}
case "groups/v1":
rawTransform = &celtransformer.GroupsTransformation{Expression: e.Expression}
case "policy/v1":
rawTransform = &celtransformer.AllowAuthenticationPolicy{
Expression: e.Expression,
RejectedAuthenticationMessage: e.Message,
}
default:
// TODO: this shouldn't really happen since the CRD validates it, but handle it as an error
}
compiledTransform, err := c.celTransformer.CompileTransformation(rawTransform, consts)
if err != nil {
// TODO: handle compile err
plog.Error("error compiling identity transformation", err,
"federationDomain", federationDomainName,
"idpDisplayName", idp.DisplayName,
"transformationIndex", idx,
"transformationType", e.Type,
"transformationExpression", e.Expression,
)
}
pipeline.AppendTransformation(compiledTransform)
plog.Debug("successfully compiled identity transformation expression",
"type", e.Type,
"expr", e.Expression,
"policyMessage", e.Message,
)
}
// Run all the provided transform examples. If any fail, put errors on the FederationDomain status.
for idx, e := range idp.Transforms.Examples {
// TODO: use a real context param below
result, _ := pipeline.Evaluate(context.TODO(), e.Username, e.Groups)
// TODO: handle err
resultWasAuthRejected := !result.AuthenticationAllowed
if e.Expects.Rejected && !resultWasAuthRejected { //nolint:gocritic,nestif
// TODO: handle this failed example
plog.Warning("FederationDomain identity provider transformations example failed: expected authentication to be rejected but it was not",
"federationDomain", federationDomainName,
"idpDisplayName", idp.DisplayName,
"exampleIndex", idx,
"expectedRejected", e.Expects.Rejected,
"actualRejectedResult", resultWasAuthRejected,
"expectedMessage", e.Expects.Message,
"actualMessageResult", result.RejectedAuthenticationMessage,
)
} else if !e.Expects.Rejected && resultWasAuthRejected {
// TODO: handle this failed example
plog.Warning("FederationDomain identity provider transformations example failed: expected authentication not to be rejected but it was rejected",
"federationDomain", federationDomainName,
"idpDisplayName", idp.DisplayName,
"exampleIndex", idx,
"expectedRejected", e.Expects.Rejected,
"actualRejectedResult", resultWasAuthRejected,
"expectedMessage", e.Expects.Message,
"actualMessageResult", 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: handle this failed example
plog.Warning("FederationDomain identity provider transformations example failed: expected a different authentication rejection message",
"federationDomain", federationDomainName,
"idpDisplayName", idp.DisplayName,
"exampleIndex", idx,
"expectedRejected", e.Expects.Rejected,
"actualRejectedResult", resultWasAuthRejected,
"expectedMessage", e.Expects.Message,
"actualMessageResult", result.RejectedAuthenticationMessage,
)
} else if result.AuthenticationAllowed {
// In the case where the user expected the auth to be allowed and it was allowed, then compare
// the expected username and group names to the actual username and group names.
// TODO: when both of these fail, put both errors onto the status (not just the first one)
if e.Expects.Username != result.Username {
// TODO: handle this failed example
plog.Warning("FederationDomain identity provider transformations example failed: expected a different transformed username",
"federationDomain", federationDomainName,
"idpDisplayName", idp.DisplayName,
"exampleIndex", idx,
"expectedUsername", e.Expects.Username,
"actualUsernameResult", result.Username,
)
}
if !stringSlicesEqual(e.Expects.Groups, result.Groups) {
// 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: handle this failed example
plog.Warning("FederationDomain identity provider transformations example failed: expected a different transformed groups list",
"federationDomain", federationDomainName,
"idpDisplayName", idp.DisplayName,
"exampleIndex", idx,
"expectedGroups", e.Expects.Groups,
"actualGroupsResult", result.Groups,
)
}
}
}
return pipeline, nil
}
func appendIssuerURLValidCondition(err error, conditions []*configv1alpha1.Condition) []*configv1alpha1.Condition {
if err != nil {
// Note that the FederationDomainIssuer constructors only validate the Issuer URL,
// so these are always issuer URL validation errors.
@ -425,32 +519,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
Message: "spec.issuer is a valid URL",
})
}
// Now that we have determined the conditions, save them for after the loop.
// For a valid FederationDomain, want to update the conditions after we have
// made the FederationDomain's endpoints available.
fdToConditionsMap[federationDomain] = conditions
if !hadErrorCondition(conditions) {
// Successfully validated the FederationDomain, so allow it to be loaded.
federationDomainIssuers = append(federationDomainIssuers, federationDomainIssuer)
}
}
// Load the endpoints of every valid FederationDomain. Removes the endpoints of any
// previous FederationDomains which no longer exist or are no longer valid.
c.federationDomainsSetter.SetFederationDomains(federationDomainIssuers...)
// Now that the endpoints of every valid FederationDomain are available, update the
// statuses. This allows clients to wait for Ready without any race conditions in the
// endpoints being available.
for federationDomain, conditions := range fdToConditionsMap {
if err = c.updateStatus(ctx.Context, federationDomain, conditions); err != nil {
errs = append(errs, fmt.Errorf("could not update status: %w", err))
}
}
return errorsutil.NewAggregate(errs)
return conditions
}
func (c *federationDomainWatcherController) updateStatus(