Handle code review changes.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
pull/597/head
Oleksandr Andriienko 2021-02-08 23:06:02 +02:00
parent 72285399fe
commit 213b10c9f5
9 changed files with 106 additions and 101 deletions

View File

@ -112,7 +112,7 @@ spec:
workspacePVCStorageClassName: ''
auth:
createOpenShiftOAuthUser: true
initialOpenShiftOAuthUser: true
# instructs operator on whether or not to deploy Keycloak/RH SSO instance. When set to true provision connection details
externalIdentityProvider: false
# retrieved from respective route/ingress unless explicitly specified in CR (when ExternalKeycloak is true)

View File

@ -12,7 +12,6 @@ metadata:
},
"spec": {
"auth": {
"createOpenShiftOAuthUser": true,
"externalIdentityProvider": false,
"identityProviderAdminUserName": "",
"identityProviderClientId": "",
@ -20,6 +19,7 @@ metadata:
"identityProviderPassword": "",
"identityProviderRealm": "",
"identityProviderURL": "",
"initialOpenShiftOAuthUser": true,
"oAuthClientName": "",
"oAuthSecret": ""
},
@ -85,13 +85,13 @@ metadata:
categories: Developer Tools
certified: "false"
containerImage: quay.io/eclipse/che-operator:nightly
createdAt: "2021-02-08T15:49:52Z"
createdAt: "2021-02-08T20:59:54Z"
description: A Kube-native development solution that delivers portable and collaborative
developer workspaces.
operatorframework.io/suggested-namespace: eclipse-che
repository: https://github.com/eclipse/che-operator
support: Eclipse Foundation
name: eclipse-che-preview-kubernetes.v7.26.0-92.nightly
name: eclipse-che-preview-kubernetes.v7.26.0-94.nightly
namespace: placeholder
spec:
apiservicedefinitions: {}
@ -686,4 +686,4 @@ spec:
maturity: stable
provider:
name: Eclipse Foundation
version: 7.26.0-92.nightly
version: 7.26.0-94.nightly

View File

@ -12,7 +12,6 @@ metadata:
},
"spec": {
"auth": {
"createOpenShiftOAuthUser": true,
"externalIdentityProvider": false,
"identityProviderAdminUserName": "",
"identityProviderClientId": "",
@ -20,6 +19,7 @@ metadata:
"identityProviderPassword": "",
"identityProviderRealm": "",
"identityProviderURL": "",
"initialOpenShiftOAuthUser": true,
"oAuthClientName": "",
"oAuthSecret": ""
},
@ -76,13 +76,13 @@ metadata:
categories: Developer Tools, OpenShift Optional
certified: "false"
containerImage: quay.io/eclipse/che-operator:nightly
createdAt: "2021-02-08T15:49:59Z"
createdAt: "2021-02-08T21:00:06Z"
description: A Kube-native development solution that delivers portable and collaborative
developer workspaces in OpenShift.
operatorframework.io/suggested-namespace: eclipse-che
repository: https://github.com/eclipse/che-operator
support: Eclipse Foundation
name: eclipse-che-preview-openshift.v7.26.0-92.nightly
name: eclipse-che-preview-openshift.v7.26.0-94.nightly
namespace: placeholder
spec:
apiservicedefinitions: {}
@ -758,4 +758,4 @@ spec:
maturity: stable
provider:
name: Eclipse Foundation
version: 7.26.0-92.nightly
version: 7.26.0-94.nightly

View File

@ -283,10 +283,10 @@ type ReconcileChe struct {
nonCachedClient client.Client
// A discovery client to check for the existence of certain APIs registered
// in the API Server
discoveryClient discovery.DiscoveryInterface
scheme *runtime.Scheme
tests bool
userHandler OpenShiftOAuthUserHandler
discoveryClient discovery.DiscoveryInterface
scheme *runtime.Scheme
tests bool
userHandler OpenShiftOAuthUserHandler
permissionChecker PermissionChecker
}
@ -384,7 +384,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e
}
}
if isOpenShift4 && !util.IsInitialOpenShiftOAuthUserEnabled(instance) && instance.Status.OpenShiftOAuthUserCredentialsSecret != "" {
if isOpenShift4 && instance.Spec.Auth.InitialOpenShiftOAuthUser != nil && !*instance.Spec.Auth.InitialOpenShiftOAuthUser && instance.Status.OpenShiftOAuthUserCredentialsSecret != "" {
if err := r.userHandler.DeleteOAuthInitialUser(instance.Namespace, deploy.DefaultCheFlavor(instance)); err != nil {
logrus.Errorf("Unable to delete initial user from cluster. Cause: %s", err.Error())
return reconcile.Result{}, err
@ -598,7 +598,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e
logrus.Warnf("Fall back to '%s' namespace for workspaces.", instance.Namespace)
delete(instance.Spec.Server.CustomCheProperties, "CHE_INFRA_KUBERNETES_NAMESPACE_DEFAULT")
instance.Spec.Server.WorkspaceNamespaceDefault = instance.Namespace
r.UpdateCheCRSpec(instance, "Default namespace for workspaces", instance.Namespace);
err := r.UpdateCheCRSpec(instance, "Default namespace for workspaces", instance.Namespace)
if err != nil {
logrus.Error(err)
return reconcile.Result{RequeueAfter: time.Second * 1}, err
@ -1134,11 +1134,17 @@ func (r *ReconcileChe) autoEnableOAuth(deployContext *deploy.DeployContext, requ
if len(openshitOAuth.Spec.IdentityProviders) > 0 {
oauth = true
} else if util.IsInitialOpenShiftOAuthUserEnabled(cr) {
if err := r.userHandler.CreateOAuthInitialUser(deploy.DefaultCheFlavor(cr), deployContext.CheCluster.Namespace, openshitOAuth); err != nil {
if err := r.userHandler.CreateOAuthInitialUser(deploy.DefaultCheFlavor(cr), cr.Namespace, openshitOAuth); err != nil {
message = warningNoIdentityProvidersMessage + " Operator tried to create initial identity provider, but failed. Cause: " + err.Error()
logrus.Warn(message)
logrus.Info(" You can create identity provider manually:" + howToAddIdentityProviderLinkOS4)
reason = failedNoIdentityProviders
// Don't try to create initial user any more, che-operator shouldn't hang on this step.
cr.Spec.Auth.InitialOpenShiftOAuthUser = nil
if err := r.UpdateCheCRStatus(cr, "initialOpenShiftOAuthUser", ""); err != nil {
return reconcile.Result{}, err
}
oauth = false
} else {
oauth = true
if deployContext.CheCluster.Status.OpenShiftOAuthUserCredentialsSecret == "" {

View File

@ -290,9 +290,9 @@ func TestCaseAutoDetectOAuth(t *testing.T) {
initObjects: []runtime.Object{
oAuthWithNoIdentityProviders,
},
openshiftVersion: "4",
initialOAuthValue: util.NewBoolPointer(true),
oAuthExpected: util.NewBoolPointer(true),
openshiftVersion: "4",
initialOAuthValue: util.NewBoolPointer(true),
oAuthExpected: util.NewBoolPointer(true),
initialOpenShiftOAuthUserEnabled: util.NewBoolPointer(true),
mockFunction: func(ctrl *gomock.Controller, crNamespace string, userNamePrefix string) *che_mocks.MockOpenShiftOAuthUserHandler {
m := che_mocks.NewMockOpenShiftOAuthUserHandler(ctrl)
@ -306,9 +306,9 @@ func TestCaseAutoDetectOAuth(t *testing.T) {
initObjects: []runtime.Object{
oAuthWithIdentityProvider,
},
openshiftVersion: "4",
initialOAuthValue: util.NewBoolPointer(true),
oAuthExpected: util.NewBoolPointer(true),
openshiftVersion: "4",
initialOAuthValue: util.NewBoolPointer(true),
oAuthExpected: util.NewBoolPointer(true),
initialOpenShiftOAuthUserEnabled: util.NewBoolPointer(true),
},
{
@ -330,12 +330,12 @@ func TestCaseAutoDetectOAuth(t *testing.T) {
oAuthExpected: util.NewBoolPointer(false),
},
{
name: "che-operator should auto disable oAuth on error retieve identity providers",
initObjects: []runtime.Object{},
openshiftVersion: "4",
initialOAuthValue: nil,
name: "che-operator should auto disable oAuth on error retieve identity providers",
initObjects: []runtime.Object{},
openshiftVersion: "4",
initialOAuthValue: nil,
initialOpenShiftOAuthUserEnabled: util.NewBoolPointer(true),
oAuthExpected: util.NewBoolPointer(false),
oAuthExpected: util.NewBoolPointer(false),
},
}
for _, testCase := range testCases {

View File

@ -1,5 +1,5 @@
//
// Copyright (c) 2012-2020 Red Hat, Inc.
// Copyright (c) 2012-2021 Red Hat, Inc.
// This program and the accompanying materials are made
// available under the terms of the Eclipse Public License 2.0
// which is available at https://www.eclipse.org/legal/epl-2.0/
@ -17,6 +17,7 @@ import (
errorMsg "errors"
"github.com/eclipse/che-operator/pkg/deploy"
"github.com/eclipse/che-operator/pkg/util"
configv1 "github.com/openshift/api/config/v1"
oauthv1 "github.com/openshift/api/config/v1"
@ -36,17 +37,20 @@ const (
openShiftOAuthUserCredentialsSecret = "openshift-oauth-user-credentials"
)
// OpenShiftOAuthUserHandler - handler to create or delete new Openshift oAuth user.
type OpenShiftOAuthUserHandler interface {
CreateOAuthInitialUser(userNamePrefix string, crNamespace string, openshiftOAuth *oauthv1.OAuth) error
DeleteOAuthInitialUser(userNamePrefix string, crNamespace string) error
CreateOAuthInitialUser(userName string, crNamespace string, openshiftOAuth *oauthv1.OAuth) error
DeleteOAuthInitialUser(userName string, crNamespace string) error
}
// OpenShiftOAuthUserOperatorHandler - implementation OpenShiftOAuthUserHandler.
type OpenShiftOAuthUserOperatorHandler struct {
OpenShiftOAuthUserHandler
runtimeClient client.Client
runnable util.Runnable
}
// NewOpenShiftOAuthUserHandler - create new OpenShiftOAuthUserHandler instance
func NewOpenShiftOAuthUserHandler(runtimeClient client.Client) OpenShiftOAuthUserHandler {
return &OpenShiftOAuthUserOperatorHandler{
runtimeClient: runtimeClient,
@ -55,26 +59,24 @@ func NewOpenShiftOAuthUserHandler(runtimeClient client.Client) OpenShiftOAuthUse
}
}
// CreateOauthInitialUser - creates new htpasswd provider with inital user with name 'che-user'
// if Openshift cluster has got no identity providers, otherwise do nothing.
// CreateOAuthInitialUser - creates new htpasswd provider with inital user with Che flavor name
// if Openshift cluster hasn't got identity providers, otherwise do nothing.
// It usefull for good first user expirience.
// User can't use kube:admin or system:admin user in the Openshift oAuth. That's why we provide
// initial user for good first meeting with Eclipse Che.
func (handler *OpenShiftOAuthUserOperatorHandler) CreateOauthInitialUser(userNamePrefix string, crNamespace string, openshiftOAuth *oauthv1.OAuth) error {
func (iuh *OpenShiftOAuthUserOperatorHandler) CreateOAuthInitialUser(userName string, crNamespace string, openshiftOAuth *oauthv1.OAuth) error {
password := util.GeneratePasswd(6)
userName := getUserName(userNamePrefix)
htpasswdFileContent, err := handler.generateHtPasswdUserInfo(userName, password)
htpasswdFileContent, err := iuh.generateHtPasswdUserInfo(userName, password)
if err != nil {
return err
}
secret := &corev1.Secret{}
nsName := types.NamespacedName{Name: htpasswdSecretName, Namespace: ocConfigNamespace}
if err := handler.runtimeClient.Get(context.TODO(), nsName, secret); err != nil {
if err := iuh.runtimeClient.Get(context.TODO(), nsName, secret); err != nil {
if errors.IsNotFound(err) {
htpasswdFileSecretData := map[string][]byte{"htpasswd": []byte(htpasswdFileContent)}
if err = createSecret(htpasswdFileSecretData, htpasswdSecretName, ocConfigNamespace, handler.runtimeClient); err != nil {
if err = deploy.CreateSecret(htpasswdFileSecretData, htpasswdSecretName, ocConfigNamespace, iuh.runtimeClient); err != nil {
return err
}
} else {
@ -82,20 +84,21 @@ func (handler *OpenShiftOAuthUserOperatorHandler) CreateOauthInitialUser(userNam
}
}
if err := appendIdentityProvider(openshiftOAuth, handler.runtimeClient); err != nil {
if err := appendIdentityProvider(openshiftOAuth, iuh.runtimeClient); err != nil {
return err
}
initialUserSecretData := map[string][]byte{"user": []byte(userName), "password": []byte(password)}
if err := createSecret(initialUserSecretData, openShiftOAuthUserCredentialsSecret, crNamespace, handler.runtimeClient); err != nil {
logrus.Info("Create initial user secret for che-operator")
if err := deploy.CreateSecret(initialUserSecretData, openShiftOAuthUserCredentialsSecret, crNamespace, iuh.runtimeClient); err != nil {
return err
}
return nil
}
// DeleteOauthInitialUser removes initial user, htpasswd provider, htpasswd secret and Che secret with username and password.
func (iuh *OpenShiftOAuthUserOperatorHandler) DeleteOauthInitialUser(userNamePrefix string, crNamespace string) error {
// DeleteOAuthInitialUser - removes initial user, htpasswd provider, htpasswd secret and Che secret with username and password.
func (iuh *OpenShiftOAuthUserOperatorHandler) DeleteOAuthInitialUser(userName string, crNamespace string) error {
oAuth, err := GetOpenshiftOAuth(iuh.runtimeClient)
if err != nil {
return err
@ -109,12 +112,11 @@ func (iuh *OpenShiftOAuthUserOperatorHandler) DeleteOauthInitialUser(userNamePre
}
if identityProviderExists {
userName := getUserName(userNamePrefix)
if err := deleteSecret(htpasswdSecretName, ocConfigNamespace, iuh.runtimeClient); err != nil {
if err := deploy.DeleteSecret(htpasswdSecretName, ocConfigNamespace, iuh.runtimeClient); err != nil {
return err
}
if err := deleteSecret(openShiftOAuthUserCredentialsSecret, crNamespace, iuh.runtimeClient); err != nil {
if err := deploy.DeleteSecret(openShiftOAuthUserCredentialsSecret, crNamespace, iuh.runtimeClient); err != nil {
return err
}
@ -134,10 +136,10 @@ func (iuh *OpenShiftOAuthUserOperatorHandler) DeleteOauthInitialUser(userNamePre
return nil
}
func (iuh *OpenShiftOAuthUserOperatorHandler) generateHtPasswdUserInfo(username string, password string) (string, error) {
func (iuh *OpenShiftOAuthUserOperatorHandler) generateHtPasswdUserInfo(userName string, password string) (string, error) {
logrus.Info("Generate initial user httpasswd info")
err := iuh.runnable.Run("htpasswd", "-nbB", username, password)
err := iuh.runnable.Run("htpasswd", "-nbB", userName, password)
if err != nil {
return "", err
}
@ -157,25 +159,6 @@ func GetOpenshiftOAuth(runtimeClient client.Client) (*oauthv1.OAuth, error) {
return oAuth, nil
}
func createSecret(content map[string][]byte, secretName string, namespace string, runtimeClient client.Client) error {
logrus.Info("Create initial user secret for che-operator")
secret := &corev1.Secret{
TypeMeta: metav1.TypeMeta{
Kind: "Secret",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Namespace: namespace,
},
Data: content,
}
if err := runtimeClient.Create(context.TODO(), secret); err != nil {
return err
}
return nil
}
func appendIdentityProvider(oAuth *oauthv1.OAuth, runtimeClient client.Client) error {
logrus.Info("Add initial user httpasswd provider to the oAuth")
@ -202,26 +185,6 @@ func newHtpasswdProvider() *oauthv1.IdentityProvider {
}
}
func deleteSecret(secretName string, namespace string, runtimeClient client.Client) error {
logrus.Infof("Delete secret: %s in the namespace: %s", secretName, namespace)
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Namespace: namespace,
},
}
if err := runtimeClient.Delete(context.TODO(), secret); err != nil {
if errors.IsNotFound(err) {
return nil
}
return err
}
return nil
}
func deleteInitialUser(runtimeClient client.Client, userName string) error {
logrus.Infof("Delete initial user: %s", userName)
@ -280,7 +243,3 @@ func deleteIdentityProvider(oAuth *configv1.OAuth, runtimeClient client.Client)
return nil
}
func getUserName(userNamePrefix string) string {
return userNamePrefix + "-user"
}

View File

@ -34,8 +34,8 @@ import (
)
const (
testNamespace = "test-namespace"
testUserNamePrefix = "test"
testNamespace = "test-namespace"
testUserName = "test"
)
func TestCreateInitialUser(t *testing.T) {
@ -72,7 +72,7 @@ func TestCreateInitialUser(t *testing.T) {
runtimeClient: runtimeClient,
runnable: m,
}
err := initialUserHandler.CreateOauthInitialUser(testUserNamePrefix, testNamespace, oAuth)
err := initialUserHandler.CreateOAuthInitialUser(testUserName, testNamespace, oAuth)
if err != nil {
t.Errorf("Failed to create user: %s", err.Error())
}
@ -100,7 +100,6 @@ func TestCreateInitialUser(t *testing.T) {
func TestDeleteInitialUser(t *testing.T) {
logf.SetLogger(zap.LoggerTo(os.Stdout, true))
userName := getUserName(testUserNamePrefix)
scheme := scheme.Scheme
orgv1.SchemeBuilder.AddToScheme(scheme)
@ -138,12 +137,12 @@ func TestDeleteInitialUser(t *testing.T) {
}
userIdentity := &userv1.Identity{
ObjectMeta: metav1.ObjectMeta{
Name: htpasswdIdentityProviderName + ":" + userName,
Name: htpasswdIdentityProviderName + ":" + testUserName,
},
}
user := &userv1.User{
ObjectMeta: metav1.ObjectMeta{
Name: userName,
Name: testUserName,
},
}
@ -153,7 +152,7 @@ func TestDeleteInitialUser(t *testing.T) {
runtimeClient: runtimeClient,
}
if err := initialUserHandler.DeleteOauthInitialUser(testUserNamePrefix, testNamespace); err != nil {
if err := initialUserHandler.DeleteOAuthInitialUser(testUserName, testNamespace); err != nil {
t.Errorf("Unable to delete initial user: %s", err.Error())
}
@ -168,12 +167,12 @@ func TestDeleteInitialUser(t *testing.T) {
}
expectedUserIdentity := &userv1.Identity{}
if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: htpasswdIdentityProviderName + ":" + userName}, expectedUserIdentity); !errors.IsNotFound(err) {
if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: htpasswdIdentityProviderName + ":" + testUserName}, expectedUserIdentity); !errors.IsNotFound(err) {
t.Errorf("Initial user identity should be deleted")
}
expectedUser := &userv1.User{}
if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: userName}, expectedUser); !errors.IsNotFound(err) {
if err := runtimeClient.Get(context.TODO(), types.NamespacedName{Name: testUserName}, expectedUser); !errors.IsNotFound(err) {
t.Errorf("Initial user should be deleted")
}

View File

@ -248,7 +248,7 @@ func (r *ReconcileChe) reconcileWorkspacePermissionsFinalizer(instance *orgv1.Ch
}
func getCheWorkspacesNamespacePolicy() []rbac.PolicyRule {
k8sPolicies := []rbac.PolicyRule{
k8sPolicies := []rbac.PolicyRule{
{
APIGroups: []string{""},
Resources: []string{"namespaces"},
@ -256,7 +256,7 @@ func getCheWorkspacesNamespacePolicy() []rbac.PolicyRule {
},
}
openshiftPolicies := []rbac.PolicyRule{
openshiftPolicies := []rbac.PolicyRule{
{
APIGroups: []string{"project.openshift.io"},
Resources: []string{"projectrequests"},

View File

@ -15,6 +15,7 @@ import (
"context"
"fmt"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"github.com/google/go-cmp/cmp"
@ -132,3 +133,43 @@ func CreateTLSSecretFromEndpoint(deployContext *DeployContext, url string, name
return nil
}
// DeleteSecret - delete secret by name and namespace
func DeleteSecret(secretName string, namespace string, runtimeClient client.Client) error {
logrus.Infof("Delete secret: %s in the namespace: %s", secretName, namespace)
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Namespace: namespace,
},
}
if err := runtimeClient.Delete(context.TODO(), secret); err != nil {
if errors.IsNotFound(err) {
return nil
}
return err
}
return nil
}
// CreateSecret - create secret by name and namespace
func CreateSecret(content map[string][]byte, secretName string, namespace string, runtimeClient client.Client) error {
secret := &corev1.Secret{
TypeMeta: metav1.TypeMeta{
Kind: "Secret",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Namespace: namespace,
},
Data: content,
}
if err := runtimeClient.Create(context.TODO(), secret); err != nil {
return err
}
return nil
}