Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Refactoring OpenShiftOAuth #1167

Merged
merged 1 commit into from
Nov 10, 2021
Merged

chore: Refactoring OpenShiftOAuth #1167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions api/v1/checluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,3 +794,7 @@ func (c *CheCluster) IsOpenShiftOAuthUserMustBeDeleted() bool {
func (c *CheCluster) IsOpenShiftOAuthEnabled() bool {
return c.Spec.Auth.OpenShiftoAuth != nil && *c.Spec.Auth.OpenShiftoAuth
}

func (c *CheCluster) IsNativeUserModeEnabled() bool {
return c.Spec.Auth.NativeUserMode != nil && *c.Spec.Auth.NativeUserMode
}
106 changes: 10 additions & 96 deletions controllers/che/checluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ package che

import (
"context"
"reflect"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -50,22 +48,10 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"

orgv1 "github.com/eclipse-che/che-operator/api/v1"
userv1 "github.com/openshift/api/user/v1"
networking "k8s.io/api/networking/v1"
"k8s.io/apimachinery/pkg/api/errors"
)

const (
warningNoIdentityProvidersMessage = "No Openshift identity providers."

AddIdentityProviderMessage = "Openshift oAuth was disabled. How to add identity provider read in the Help Link:"
warningNoRealUsersMessage = "No real users. Openshift oAuth was disabled. How to add new user read in the Help Link:"
failedUnableToGetOpenshiftUsers = "Unable to get users on the OpenShift cluster."

howToAddIdentityProviderLinkOS4 = "https://docs.openshift.com/container-platform/latest/authentication/understanding-identity-provider.html#identity-provider-overview_understanding-identity-provider"
howToConfigureOAuthLinkOS3 = "https://docs.openshift.com/container-platform/3.11/install_config/configuring_authentication.html"
)

// CheClusterReconciler reconciles a CheCluster object
type CheClusterReconciler struct {
Log logr.Logger
Expand All @@ -82,10 +68,9 @@ type CheClusterReconciler struct {
nonCachedClient client.Client
// A discovery client to check for the existence of certain APIs registered
// in the API Server
discoveryClient discovery.DiscoveryInterface
tests bool
openShiftOAuthUser openshiftoauth.IOpenShiftOAuthUser
reconcileManager *deploy.ReconcileManager
discoveryClient discovery.DiscoveryInterface
tests bool
reconcileManager *deploy.ReconcileManager
// the namespace to which to limit the reconciliation. If empty, all namespaces are considered
namespace string
}
Expand All @@ -108,17 +93,17 @@ func NewReconciler(

openShiftOAuthUser := openshiftoauth.NewOpenShiftOAuthUser()
reconcileManager.RegisterReconciler(openShiftOAuthUser)
reconcileManager.RegisterReconciler(openshiftoauth.NewOpenShiftOAuth(openShiftOAuthUser))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have a dependency between these reconcilers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is dependency between class.
Because they actually depends on each other.


return &CheClusterReconciler{
Scheme: scheme,
Log: ctrl.Log.WithName("controllers").WithName("CheCluster"),

client: k8sclient,
nonCachedClient: noncachedClient,
discoveryClient: discoveryClient,
openShiftOAuthUser: openShiftOAuthUser,
namespace: namespace,
reconcileManager: reconcileManager,
client: k8sclient,
nonCachedClient: noncachedClient,
discoveryClient: discoveryClient,
namespace: namespace,
reconcileManager: reconcileManager,
}
}

Expand Down Expand Up @@ -302,20 +287,6 @@ func (r *CheClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
// TODO remove in favor of r.reconcileManager.FinalizeAll(deployContext)
r.reconcileFinalizers(deployContext)

if util.IsOpenShift && checluster.Spec.DevWorkspace.Enable && checluster.Spec.Auth.NativeUserMode == nil {
newNativeUserModeValue := util.NewBoolPointer(true)
checluster.Spec.Auth.NativeUserMode = newNativeUserModeValue
if err := deploy.UpdateCheCRSpec(deployContext, "nativeUserMode", strconv.FormatBool(*newNativeUserModeValue)); err != nil {
return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 1}, err
}
}

if util.IsOpenShift && checluster.Spec.Auth.OpenShiftoAuth == nil {
if reconcileResult, err := r.autoEnableOAuth(deployContext); err != nil {
return reconcileResult, err
}
}

// Reconcile Dev Workspace Operator
done, err := devworkspace.ReconcileDevWorkspace(deployContext)
if !done {
Expand Down Expand Up @@ -612,71 +583,14 @@ func (r *CheClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, nil
}

func (r *CheClusterReconciler) autoEnableOAuth(deployContext *deploy.DeployContext) (reconcile.Result, error) {
oauth := false
cr := deployContext.CheCluster
if util.IsOpenShift4 {
openshitOAuth, err := openshiftoauth.GetOpenshiftOAuth(deployContext)
if err != nil {
logrus.Error("Unable to get Openshift oAuth. Cause: " + err.Error())
} else {
if len(openshitOAuth.Spec.IdentityProviders) > 0 {
oauth = true
} else if util.IsNativeUserModeEnabled(deployContext.CheCluster) {
// enable OpenShift OAuth without adding initial OpenShift OAuth user
// since kubeadmin is a valid user for native user mode
oauth = true
} else if cr.IsOpenShiftOAuthUserConfigured() {
provisioned, err := r.openShiftOAuthUser.Create(deployContext)
if err != nil {
logrus.Error(warningNoIdentityProvidersMessage + " Operator tried to create initial OpenShift OAuth user for HTPasswd identity provider, but failed. Cause: " + err.Error())
logrus.Info("To enable OpenShift OAuth, please add identity provider first: " + howToAddIdentityProviderLinkOS4)
// Don't try to create initial user any more, che-operator shouldn't hang on this step.
cr.Spec.Auth.InitialOpenShiftOAuthUser = nil
if err := deploy.UpdateCheCRStatus(deployContext, "initialOpenShiftOAuthUser", ""); err != nil {
return reconcile.Result{}, err
}
oauth = false
} else {
if !provisioned {
return reconcile.Result{}, err
}
oauth = true
}
}
}
} else { // Openshift 3
users := &userv1.UserList{}
listOptions := &client.ListOptions{}
if err := r.nonCachedClient.List(context.TODO(), users, listOptions); err != nil {
logrus.Error(failedUnableToGetOpenshiftUsers + " Cause: " + err.Error())
} else {
oauth = len(users.Items) >= 1
if !oauth {
logrus.Warn(warningNoRealUsersMessage + " " + howToConfigureOAuthLinkOS3)
}
}
}

newOAuthValue := util.NewBoolPointer(oauth)
if !reflect.DeepEqual(newOAuthValue, cr.Spec.Auth.OpenShiftoAuth) {
cr.Spec.Auth.OpenShiftoAuth = newOAuthValue
if err := deploy.UpdateCheCRSpec(deployContext, "openShiftoAuth", strconv.FormatBool(oauth)); err != nil {
return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 1}, err
}
}

return reconcile.Result{}, nil
}

func (r *CheClusterReconciler) reconcileFinalizers(deployContext *deploy.DeployContext) {
if util.IsOpenShift && deployContext.CheCluster.IsOpenShiftOAuthEnabled() {
if err := deploy.ReconcileOAuthClientFinalizer(deployContext); err != nil {
logrus.Error(err)
}
}

if util.IsNativeUserModeEnabled(deployContext.CheCluster) {
if util.IsOpenShift && deployContext.CheCluster.IsNativeUserModeEnabled() {
if _, err := r.reconcileGatewayPermissionsFinalizers(deployContext); err != nil {
logrus.Error(err)
}
Expand Down
Loading