Try to handle remaining feedback.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
pull/597/head
Oleksandr Andriienko 2021-02-09 23:12:57 +02:00
parent cada10c57c
commit 1fb898e2d2
8 changed files with 77 additions and 72 deletions

View File

@ -60,6 +60,7 @@ kubectl apply -f "${CR}" -n $CHE_NAMESPACE
cp templates/keycloak-provision.sh /tmp/keycloak-provision.sh
cp templates/delete-identity-provider.sh /tmp/delete-identity-provider.sh
cp templates/create-github-identity-provider.sh /tmp/create-github-identity-provider.sh
cp templates/oauth-provision.sh /tmp/oauth-provision.sh
ENV_FILE=/tmp/che-operator-debug.env
rm -rf "${ENV_FILE}"

View File

@ -5,6 +5,7 @@
package mock_che
import (
deploy "github.com/eclipse/che-operator/pkg/deploy"
gomock "github.com/golang/mock/gomock"
v1 "github.com/openshift/api/config/v1"
reflect "reflect"
@ -34,29 +35,29 @@ func (m *MockOpenShiftOAuthUserHandler) EXPECT() *MockOpenShiftOAuthUserHandlerM
}
// CreateOAuthInitialUser mocks base method
func (m *MockOpenShiftOAuthUserHandler) CreateOAuthInitialUser(userNamePrefix, crNamespace string, openshiftOAuth *v1.OAuth) error {
func (m *MockOpenShiftOAuthUserHandler) CreateOAuthInitialUser(openshiftOAuth *v1.OAuth, deployContext *deploy.DeployContext) error {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "CreateOAuthInitialUser", userNamePrefix, crNamespace, openshiftOAuth)
ret := m.ctrl.Call(m, "CreateOAuthInitialUser", openshiftOAuth, deployContext)
ret0, _ := ret[0].(error)
return ret0
}
// CreateOAuthInitialUser indicates an expected call of CreateOAuthInitialUser
func (mr *MockOpenShiftOAuthUserHandlerMockRecorder) CreateOAuthInitialUser(userNamePrefix, crNamespace, openshiftOAuth interface{}) *gomock.Call {
func (mr *MockOpenShiftOAuthUserHandlerMockRecorder) CreateOAuthInitialUser(openshiftOAuth, deployContext interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOAuthInitialUser", reflect.TypeOf((*MockOpenShiftOAuthUserHandler)(nil).CreateOAuthInitialUser), userNamePrefix, crNamespace, openshiftOAuth)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOAuthInitialUser", reflect.TypeOf((*MockOpenShiftOAuthUserHandler)(nil).CreateOAuthInitialUser), openshiftOAuth, deployContext)
}
// DeleteOAuthInitialUser mocks base method
func (m *MockOpenShiftOAuthUserHandler) DeleteOAuthInitialUser(userNamePrefix, crNamespace string) error {
func (m *MockOpenShiftOAuthUserHandler) DeleteOAuthInitialUser(deployContext *deploy.DeployContext) error {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "DeleteOAuthInitialUser", userNamePrefix, crNamespace)
ret := m.ctrl.Call(m, "DeleteOAuthInitialUser", deployContext)
ret0, _ := ret[0].(error)
return ret0
}
// DeleteOAuthInitialUser indicates an expected call of DeleteOAuthInitialUser
func (mr *MockOpenShiftOAuthUserHandlerMockRecorder) DeleteOAuthInitialUser(userNamePrefix, crNamespace interface{}) *gomock.Call {
func (mr *MockOpenShiftOAuthUserHandlerMockRecorder) DeleteOAuthInitialUser(deployContext interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOAuthInitialUser", reflect.TypeOf((*MockOpenShiftOAuthUserHandler)(nil).DeleteOAuthInitialUser), userNamePrefix, crNamespace)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOAuthInitialUser", reflect.TypeOf((*MockOpenShiftOAuthUserHandler)(nil).DeleteOAuthInitialUser), deployContext)
}

View File

@ -385,7 +385,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e
}
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 {
if err := r.userHandler.DeleteOAuthInitialUser(deployContext); err != nil {
logrus.Errorf("Unable to delete initial user from cluster. Cause: %s", err.Error())
instance.Spec.Auth.InitialOpenShiftOAuthUser = nil
if err := r.UpdateCheCRSpec(instance, "InitialOpenShiftOAuthUser", "nil"); err != nil {
@ -1137,7 +1137,7 @@ 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), cr.Namespace, openshitOAuth); err != nil {
if err := r.userHandler.CreateOAuthInitialUser(openshitOAuth, deployContext); err != nil {
message = warningNoIdentityProvidersMessage + " Operator tried to create initial OpenShift OAuth user for HTPasswd identity provider, but failed. Cause: " + err.Error()
logrus.Error(message)
logrus.Info("To enable OpenShift OAuth, please add identity provider first: " + howToAddIdentityProviderLinkOS4)

View File

@ -296,7 +296,7 @@ func TestCaseAutoDetectOAuth(t *testing.T) {
initialOpenShiftOAuthUserEnabled: util.NewBoolPointer(true),
mockFunction: func(ctrl *gomock.Controller, crNamespace string, userNamePrefix string) *che_mocks.MockOpenShiftOAuthUserHandler {
m := che_mocks.NewMockOpenShiftOAuthUserHandler(ctrl)
m.EXPECT().CreateOAuthInitialUser(userNamePrefix, crNamespace, gomock.Any())
m.EXPECT().CreateOAuthInitialUser(gomock.Any(), gomock.Any())
return m
},
OpenShiftOAuthUserCredentialsSecret: "openshift-oauth-user-credentials",

View File

@ -19,6 +19,7 @@ import (
func (r *ReconcileChe) GenerateAndSaveFields(deployContext *deploy.DeployContext, request reconcile.Request) (err error) {
cheFlavor := deploy.DefaultCheFlavor(deployContext.CheCluster)
cheNamespace := deployContext.CheCluster.Namespace
if len(deployContext.CheCluster.Spec.Server.CheFlavor) < 1 {
deployContext.CheCluster.Spec.Server.CheFlavor = cheFlavor
if err := r.UpdateCheCRSpec(deployContext.CheCluster, "installation flavor", cheFlavor); err != nil {
@ -31,7 +32,10 @@ func (r *ReconcileChe) GenerateAndSaveFields(deployContext *deploy.DeployContext
if len(deployContext.CheCluster.Spec.Database.ChePostgresSecret) < 1 {
if len(deployContext.CheCluster.Spec.Database.ChePostgresUser) < 1 || len(deployContext.CheCluster.Spec.Database.ChePostgresPassword) < 1 {
chePostgresSecret := deploy.DefaultChePostgresSecret()
deploy.SyncSecretToCluster(deployContext, chePostgresSecret, map[string][]byte{"user": []byte(deploy.DefaultChePostgresUser), "password": []byte(util.GeneratePasswd(12))})
_, err := deploy.SyncSecret(deployContext, chePostgresSecret, cheNamespace, map[string][]byte{"user": []byte(deploy.DefaultChePostgresUser), "password": []byte(util.GeneratePasswd(12))})
if err != nil {
return err
}
deployContext.CheCluster.Spec.Database.ChePostgresSecret = chePostgresSecret
if err := r.UpdateCheCRSpec(deployContext.CheCluster, "Postgres Secret", chePostgresSecret); err != nil {
return err
@ -60,7 +64,10 @@ func (r *ReconcileChe) GenerateAndSaveFields(deployContext *deploy.DeployContext
if len(deployContext.CheCluster.Spec.Auth.IdentityProviderPostgresPassword) < 1 {
identityPostgresSecret := deploy.DefaultCheIdentityPostgresSecret()
deploy.SyncSecretToCluster(deployContext, identityPostgresSecret, map[string][]byte{"password": []byte(keycloakPostgresPassword)})
_, err := deploy.SyncSecret(deployContext, identityPostgresSecret, cheNamespace, map[string][]byte{"password": []byte(keycloakPostgresPassword)})
if err != nil {
return err
}
deployContext.CheCluster.Spec.Auth.IdentityProviderPostgresSecret = identityPostgresSecret
if err := r.UpdateCheCRSpec(deployContext.CheCluster, "Identity Provider Postgres Secret", identityPostgresSecret); err != nil {
return err
@ -80,7 +87,7 @@ func (r *ReconcileChe) GenerateAndSaveFields(deployContext *deploy.DeployContext
if len(deployContext.CheCluster.Spec.Auth.IdentityProviderAdminUserName) < 1 || len(deployContext.CheCluster.Spec.Auth.IdentityProviderPassword) < 1 {
identityProviderSecret := deploy.DefaultCheIdentitySecret()
_, err = deploy.SyncSecretToCluster(deployContext, identityProviderSecret, map[string][]byte{"user": []byte(keycloakAdminUserName), "password": []byte(keycloakAdminPassword)})
_, err = deploy.SyncSecret(deployContext, identityProviderSecret, cheNamespace, map[string][]byte{"user": []byte(keycloakAdminUserName), "password": []byte(keycloakAdminPassword)})
if err != nil {
return err
}

View File

@ -23,7 +23,6 @@ import (
oauthv1 "github.com/openshift/api/config/v1"
userv1 "github.com/openshift/api/user/v1"
"github.com/sirupsen/logrus"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
@ -39,8 +38,8 @@ const (
// OpenShiftOAuthUserHandler - handler to create or delete new Openshift oAuth user.
type OpenShiftOAuthUserHandler interface {
CreateOAuthInitialUser(userName string, crNamespace string, openshiftOAuth *oauthv1.OAuth) error
DeleteOAuthInitialUser(userName string, crNamespace string) error
CreateOAuthInitialUser(openshiftOAuth *oauthv1.OAuth, deployContext *deploy.DeployContext) error
DeleteOAuthInitialUser(deployContext *deploy.DeployContext) error
}
// OpenShiftOAuthUserOperatorHandler - implementation OpenShiftOAuthUserHandler.
@ -64,24 +63,18 @@ func NewOpenShiftOAuthUserHandler(runtimeClient client.Client) OpenShiftOAuthUse
// 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 (iuh *OpenShiftOAuthUserOperatorHandler) CreateOAuthInitialUser(userName string, crNamespace string, openshiftOAuth *oauthv1.OAuth) error {
func (iuh *OpenShiftOAuthUserOperatorHandler) CreateOAuthInitialUser(openshiftOAuth *oauthv1.OAuth, deployContext *deploy.DeployContext) error {
password := util.GeneratePasswd(6)
cr := deployContext.CheCluster
userName := deploy.DefaultCheFlavor(cr)
htpasswdFileContent, err := iuh.generateHtPasswdUserInfo(userName, password)
if err != nil {
return err
}
secret := &corev1.Secret{}
nsName := types.NamespacedName{Name: htpasswdSecretName, Namespace: ocConfigNamespace}
if err := iuh.runtimeClient.Get(context.TODO(), nsName, secret); err != nil {
if errors.IsNotFound(err) {
htpasswdFileSecretData := map[string][]byte{"htpasswd": []byte(htpasswdFileContent)}
if err = deploy.CreateSecret(htpasswdFileSecretData, htpasswdSecretName, ocConfigNamespace, iuh.runtimeClient); err != nil {
return err
}
} else {
return err
}
htpasswdFileSecretData := map[string][]byte{"htpasswd": []byte(htpasswdFileContent)}
if _, err := deploy.SyncSecret(deployContext, htpasswdSecretName, ocConfigNamespace, htpasswdFileSecretData); err != nil {
return err
}
if err := appendIdentityProvider(openshiftOAuth, iuh.runtimeClient); err != nil {
@ -90,7 +83,7 @@ func (iuh *OpenShiftOAuthUserOperatorHandler) CreateOAuthInitialUser(userName st
initialUserSecretData := map[string][]byte{"user": []byte(userName), "password": []byte(password)}
logrus.Info("Create initial user secret for che-operator")
if err := deploy.CreateSecret(initialUserSecretData, openShiftOAuthUserCredentialsSecret, crNamespace, iuh.runtimeClient); err != nil {
if _, err := deploy.SyncSecret(deployContext, openShiftOAuthUserCredentialsSecret, cr.Namespace, initialUserSecretData); err != nil {
return err
}
@ -98,25 +91,20 @@ func (iuh *OpenShiftOAuthUserOperatorHandler) CreateOAuthInitialUser(userName st
}
// DeleteOAuthInitialUser - removes initial user, htpasswd provider, htpasswd secret and Che secret with username and password.
func (iuh *OpenShiftOAuthUserOperatorHandler) DeleteOAuthInitialUser(userName string, crNamespace string) error {
func (iuh *OpenShiftOAuthUserOperatorHandler) DeleteOAuthInitialUser(deployContext *deploy.DeployContext) error {
oAuth, err := GetOpenshiftOAuth(iuh.runtimeClient)
if err != nil {
return err
}
var identityProviderExists bool
for _, ip := range oAuth.Spec.IdentityProviders {
if ip.Name == htpasswdIdentityProviderName {
identityProviderExists = true
break
}
}
if identityProviderExists {
if identityProviderExists(htpasswdIdentityProviderName, oAuth) {
cr := deployContext.CheCluster
userName := deploy.DefaultCheFlavor(cr)
if err := deploy.DeleteSecret(htpasswdSecretName, ocConfigNamespace, iuh.runtimeClient); err != nil {
return err
}
if err := deploy.DeleteSecret(openShiftOAuthUserCredentialsSecret, crNamespace, iuh.runtimeClient); err != nil {
if err := deploy.DeleteSecret(openShiftOAuthUserCredentialsSecret, cr.Namespace, iuh.runtimeClient); err != nil {
return err
}

View File

@ -14,8 +14,12 @@ package che
import (
"context"
"os"
"testing"
util_mocks "github.com/eclipse/che-operator/mocks/pkg/util"
orgv1 "github.com/eclipse/che-operator/pkg/apis/org/v1"
"github.com/eclipse/che-operator/pkg/deploy"
"github.com/golang/mock/gomock"
oauth_config "github.com/openshift/api/config/v1"
userv1 "github.com/openshift/api/user/v1"
@ -26,11 +30,9 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
"os"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
"testing"
)
const (
@ -38,6 +40,20 @@ const (
testUserName = "test"
)
var (
testCR = &orgv1.CheCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eclipse-che",
Namespace: testNamespace,
},
Spec: orgv1.CheClusterSpec{
Server: orgv1.CheClusterSpecServer{
CheFlavor: testUserName,
},
},
}
)
func TestCreateInitialUser(t *testing.T) {
type testCase struct {
name string
@ -72,7 +88,11 @@ func TestCreateInitialUser(t *testing.T) {
runtimeClient: runtimeClient,
runnable: m,
}
err := initialUserHandler.CreateOAuthInitialUser(testUserName, testNamespace, oAuth)
dc := &deploy.DeployContext{
CheCluster: testCR,
ClusterAPI: deploy.ClusterAPI{Client: runtimeClient, NonCachedClient: runtimeClient, DiscoveryClient: nil, Scheme: scheme},
}
err := initialUserHandler.CreateOAuthInitialUser(oAuth, dc)
if err != nil {
t.Errorf("Failed to create user: %s", err.Error())
}
@ -152,7 +172,11 @@ func TestDeleteInitialUser(t *testing.T) {
runtimeClient: runtimeClient,
}
if err := initialUserHandler.DeleteOAuthInitialUser(testUserName, testNamespace); err != nil {
dc := &deploy.DeployContext{
CheCluster: testCR,
ClusterAPI: deploy.ClusterAPI{Client: runtimeClient, NonCachedClient: runtimeClient, DiscoveryClient: nil, Scheme: scheme},
}
if err := initialUserHandler.DeleteOAuthInitialUser(dc); err != nil {
t.Errorf("Unable to delete initial user: %s", err.Error())
}

View File

@ -31,13 +31,14 @@ var secretDiffOpts = cmp.Options{
cmpopts.IgnoreFields(corev1.Secret{}, "TypeMeta", "ObjectMeta"),
}
// SyncSecretToCluster applies secret into cluster
func SyncSecretToCluster(
// SyncSecret applies secret into cluster or external namespace
func SyncSecret(
deployContext *DeployContext,
name string,
namespace string,
data map[string][]byte) (*corev1.Secret, error) {
specSecret, err := GetSpecSecret(deployContext, name, data)
specSecret, err := GetSpecSecret(deployContext, name, namespace, data)
if err != nil {
return nil, err
}
@ -90,7 +91,7 @@ func GetClusterSecret(name string, namespace string, clusterAPI ClusterAPI) (*co
}
// GetSpecSecret return default secret config for given data
func GetSpecSecret(deployContext *DeployContext, name string, data map[string][]byte) (*corev1.Secret, error) {
func GetSpecSecret(deployContext *DeployContext, name string, namespace string, data map[string][]byte) (*corev1.Secret, error) {
labels := GetLabels(deployContext.CheCluster, DefaultCheFlavor(deployContext.CheCluster))
secret := &corev1.Secret{
TypeMeta: metav1.TypeMeta{
@ -99,15 +100,17 @@ func GetSpecSecret(deployContext *DeployContext, name string, data map[string][]
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: deployContext.CheCluster.Namespace,
Namespace: namespace,
Labels: labels,
},
Data: data,
}
err := controllerutil.SetControllerReference(deployContext.CheCluster, secret, deployContext.ClusterAPI.Scheme)
if err != nil {
return nil, err
if deployContext.CheCluster.Namespace == namespace {
err := controllerutil.SetControllerReference(deployContext.CheCluster, secret, deployContext.ClusterAPI.Scheme)
if err != nil {
return nil, err
}
}
return secret, nil
@ -125,7 +128,7 @@ func CreateTLSSecretFromEndpoint(deployContext *DeployContext, url string, name
return err
}
secret, err = SyncSecretToCluster(deployContext, name, map[string][]byte{"ca.crt": crtBytes})
secret, err = SyncSecret(deployContext, name, deployContext.CheCluster.Namespace, map[string][]byte{"ca.crt": crtBytes})
if err != nil {
return err
}
@ -154,22 +157,3 @@ func DeleteSecret(secretName string, namespace string, runtimeClient client.Clie
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
}