Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions manifests/08_clusteroperator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,6 @@ status:
- group: config.openshift.io
name: cluster
resource: oauths
- group: route.openshift.io
name: oauth-openshift
namespace: openshift-authentication
resource: routes
- group: ""
name: oauth-openshift
namespace: openshift-authentication
resource: services
- group: ""
name: openshift-config
resource: namespaces
Expand Down
24 changes: 20 additions & 4 deletions pkg/operator/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,27 @@ func prepareOauthAPIServerOperator(
statusControllerOptions = append(statusControllerOptions, apiservercontrollerset.WithStatusControllerPdbCompatibleHighInertia("(APIServer|OAuthServer)"))
}

// configure version removal so it removes versions it doesn't know about.
statusControllerOptions = append(statusControllerOptions, func(ss *status.StatusSyncer) *status.StatusSyncer {
return ss.WithVersionRemoval()
// configure version removal so it removes versions it doesn't know about.
s := ss.WithVersionRemoval()

// configure func to dynamically determine oauth-specific relatedObjects
s.WithRelatedObjectsFunc(func() (isset bool, objs []configv1.ObjectReference) {
oidcAvailable, err := authConfigChecker.OIDCAvailable()
if err != nil {
klog.Infof("error while checking auth config to determine relatedObjects: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log as an error?

return false, nil
} else if oidcAvailable {
return true, nil
}

return true, []configv1.ObjectReference{
{Group: routev1.GroupName, Resource: "routes", Name: "oauth-openshift", Namespace: "openshift-authentication"},
{Resource: "services", Name: "oauth-openshift", Namespace: "openshift-authentication"},
}
})

return s
})

const apiServerConditionsPrefix = "APIServer"
Expand Down Expand Up @@ -622,8 +640,6 @@ func prepareOauthAPIServerOperator(
{Group: configv1.GroupName, Resource: "authentications", Name: "cluster"},
{Group: configv1.GroupName, Resource: "infrastructures", Name: "cluster"},
{Group: configv1.GroupName, Resource: "oauths", Name: "cluster"},
{Group: routev1.GroupName, Resource: "routes", Name: "oauth-openshift", Namespace: "openshift-authentication"},
{Resource: "services", Name: "oauth-openshift", Namespace: "openshift-authentication"},
{Resource: "namespaces", Name: "openshift-config"},
{Resource: "namespaces", Name: "openshift-config-managed"},
{Resource: "namespaces", Name: "openshift-authentication"},
Expand Down
38 changes: 38 additions & 0 deletions test/e2e-oidc/external_oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
configv1 "github.com/openshift/api/config/v1"
"github.com/openshift/api/features"
operatorv1 "github.com/openshift/api/operator/v1"
routev1 "github.com/openshift/api/route/v1"
configclient "github.com/openshift/client-go/config/clientset/versioned"
oauthclient "github.com/openshift/client-go/oauth/clientset/versioned"
operatorversionedclient "github.com/openshift/client-go/operator/clientset/versioned"
Expand Down Expand Up @@ -728,6 +729,7 @@ func (tc *testClient) validateOAuthState(t *testing.T, ctx context.Context, requ
validationErrs = append(validationErrs, validateOAuthRoutes(ctx, tc.routeClient, tc.configClient, requireMissing)...)
validationErrs = append(validationErrs, validateOAuthControllerConditions(tc.operatorClient, requireMissing)...)
validationErrs = append(validationErrs, validateOperandVersions(ctx, tc.configClient, requireMissing)...)
validationErrs = append(validationErrs, validateOAuthRelatedObjects(ctx, tc.configClient, requireMissing)...)
return len(validationErrs) == 0, nil
})

Expand Down Expand Up @@ -900,6 +902,42 @@ func validateOperandVersions(ctx context.Context, cfgClient *configclient.Client
return nil
}

func validateOAuthRelatedObjects(ctx context.Context, configClient *configclient.Clientset, requireMissing bool) []error {
co, err := configClient.ConfigV1().ClusterOperators().Get(ctx, "authentication", metav1.GetOptions{})
if err != nil {
return []error{err}
}

oauthRelatedObjects := []configv1.ObjectReference{
{Group: routev1.GroupName, Resource: "routes", Name: "oauth-openshift", Namespace: "openshift-authentication"},
{Resource: "services", Name: "oauth-openshift", Namespace: "openshift-authentication"},
}

errs := make([]error, 0)
for _, oauthObj := range oauthRelatedObjects {
found := false
for _, existingObj := range co.Status.RelatedObjects {
if oauthObj.Group == existingObj.Group &&
oauthObj.Resource == existingObj.Resource &&
oauthObj.Name == existingObj.Name &&
oauthObj.Namespace == existingObj.Namespace {
found = true
break
}
}

if requireMissing && found {
errs = append(errs, fmt.Errorf("oauth related object %s/%s %s/%s should be missing but was found in RelatedObjects",
oauthObj.Group, oauthObj.Resource, oauthObj.Namespace, oauthObj.Name))
} else if !requireMissing && !found {
errs = append(errs, fmt.Errorf("oauth related object %s/%s %s/%s should be present but was not found in RelatedObjects",
oauthObj.Group, oauthObj.Resource, oauthObj.Namespace, oauthObj.Name))
}
}

return errs
}

func (tc *testClient) testOIDCAuthentication(t *testing.T, ctx context.Context, kcClient *test.KeycloakClient, usernameClaim, usernamePrefix string, expectAuthSuccess bool) {
// re-authenticate to ensure we always have a fresh token
var err error
Expand Down