seg_yinzy
/
che-operator
mirror of https://github.com/eclipse-che/che-operator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: Get OAuthClient by its name instead of label selector (#1428) 

* fix: Get OAuthClient

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
pull/1432/head
Anatolii Bazko 2022-07-02 14:08:02 +03:00 committed by GitHub
parent 86555caf3b
commit b363587f45
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 133 additions and 149 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/deploy/gateway/oauth_proxy.go
View File

@ -61,7 +61,7 @@ func openshiftOauthProxyConfig(ctx *chetypes.DeployContext, cookieSecret string)
oauthSecret := ""
oauthClientName := ""
oauthClient, _ := identityprovider.FindOAuthClient(ctx)
oauthClient, _ := identityprovider.GetOAuthClient(ctx)
if oauthClient == nil {
logrus.Error("oauth client not found")
} else {

52
pkg/deploy/identity-provider/identity_provider_reconciler.go
View File

@ -12,9 +12,8 @@
package identityprovider
import (
"strings"
"github.com/eclipse-che/che-operator/pkg/common/utils"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"github.com/eclipse-che/che-operator/pkg/common/chetypes"
@ -46,57 +45,38 @@ func (ip *IdentityProviderReconciler) Reconcile(ctx *chetypes.DeployContext) (re
}
func (ip *IdentityProviderReconciler) Finalize(ctx *chetypes.DeployContext) bool {
oauthClients, err := FindAllEclipseCheOAuthClients(ctx)
oauthClient, err := GetOAuthClient(ctx)
if err != nil {
logrus.Errorf("Error getting OAuthClients: %v", err)
return false
}
for _, oauthClient := range oauthClients {
if _, err := deploy.DeleteClusterObject(ctx, oauthClient.Name, &oauth.OAuthClient{}); err != nil {
if oauthClient != nil {
if err := deploy.DeleteObjectWithFinalizer(ctx, types.NamespacedName{Name: oauthClient.Name}, &oauth.OAuthClient{}, OAuthFinalizerName); err != nil {
logrus.Errorf("Error deleting OAuthClient: %v", err)
return false
}
}
if err := deploy.DeleteFinalizer(ctx, OAuthFinalizerName); err != nil {
logrus.Errorf("Error deleting finalizer: %v", err)
return false
}
return true
}
func syncOAuthClient(ctx *chetypes.DeployContext) (bool, error) {
oauthClientName := ctx.CheCluster.Spec.Networking.Auth.OAuthClientName
oauthSecret := ctx.CheCluster.Spec.Networking.Auth.OAuthSecret
var oauthClientName, oauthSecret string
if oauthClientName == "" {
oauthClient, err := FindOAuthClient(ctx)
if err != nil {
logrus.Errorf("Error getting OAuthClients: %v", err)
return false, err
}
if oauthClient != nil {
oauthClientName = oauthClient.Name
if oauthSecret == "" {
oauthSecret = oauthClient.Secret
}
}
} else {
oauthClient := &oauth.OAuthClient{}
exists, _ := deploy.GetClusterObject(ctx, oauthClientName, oauthClient)
if exists {
if oauthSecret == "" {
oauthSecret = oauthClient.Secret
}
}
oauthClient, err := GetOAuthClient(ctx)
if err != nil {
logrus.Errorf("Error getting OAuthClients: %v", err)
return false, err
}
// Generate secret and name
oauthSecret = utils.GetValue(oauthSecret, utils.GeneratePassword(12))
oauthClientName = utils.GetValue(oauthClientName, ctx.CheCluster.Name+"-openshift-identity-provider-"+strings.ToLower(utils.GeneratePassword(6)))
if oauthClient != nil {
oauthClientName = oauthClient.Name
oauthSecret = utils.GetValue(ctx.CheCluster.Spec.Networking.Auth.OAuthSecret, oauthClient.Secret)
} else {
oauthClientName = GetOAuthClientName(ctx)
oauthSecret = utils.GetValue(ctx.CheCluster.Spec.Networking.Auth.OAuthSecret, utils.GeneratePassword(12))
}
redirectURIs := []string{"https://" + ctx.CheHost + "/oauth/callback"}
oauthClientSpec := GetOAuthClientSpec(

188
pkg/deploy/identity-provider/identity_provider_reconciler_test.go
View File

@ -28,11 +28,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log/zap"
)
func TestFinalize(t *testing.T) {
oauthClient1 := GetOAuthClientSpec("test1", "secret", []string{"https://che-host/oauth/callback"}, nil, nil)
oauthClient2 := GetOAuthClientSpec("test2", "secret", []string{"https://che-host/oauth/callback"}, nil, nil)
oauthClient3 := GetOAuthClientSpec("test3", "secret", []string{"https://che-host/oauth/callback"}, nil, nil)
oauthClient3.ObjectMeta.Labels = map[string]string{}
func TestFinalizeDefaultOAuthClientName(t *testing.T) {
oauthClient := GetOAuthClientSpec("eclipse-che-client", "secret", []string{"https://che-host/oauth/callback"}, nil, nil)
oauthClient.ObjectMeta.Labels = map[string]string{}
checluster := &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
@ -42,21 +40,72 @@ func TestFinalize(t *testing.T) {
},
}
ctx := test.GetDeployContext(checluster, []runtime.Object{oauthClient1, oauthClient2, oauthClient3})
ctx := test.GetDeployContext(checluster, []runtime.Object{oauthClient})
assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "test1"}, &oauthv1.OAuthClient{}))
assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "test2"}, &oauthv1.OAuthClient{}))
assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "test3"}, &oauthv1.OAuthClient{}))
assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "eclipse-che-client"}, &oauthv1.OAuthClient{}))
identityProviderReconciler := NewIdentityProviderReconciler()
done := identityProviderReconciler.Finalize(ctx)
assert.True(t, done)
assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "test1"}, &oauthv1.OAuthClient{}))
assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "test2"}, &oauthv1.OAuthClient{}))
assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "test3"}, &oauthv1.OAuthClient{}))
assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "eclipse-che-client"}, &oauthv1.OAuthClient{}))
assert.Equal(t, 0, len(checluster.Finalizers))
}
func TestFinalizeOAuthClient(t *testing.T) {
oauthClient := GetOAuthClientSpec("test", "secret", []string{"https://che-host/oauth/callback"}, nil, nil)
oauthClient.ObjectMeta.Labels = map[string]string{}
checluster := &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eclipse-che",
Namespace: "eclipse-che",
Finalizers: []string{OAuthFinalizerName},
},
Spec: chev2.CheClusterSpec{
Networking: chev2.CheClusterSpecNetworking{
Auth: chev2.Auth{
OAuthClientName: "test",
},
},
},
}
ctx := test.GetDeployContext(checluster, []runtime.Object{oauthClient})
assert.True(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "test"}, &oauthv1.OAuthClient{}))
identityProviderReconciler := NewIdentityProviderReconciler()
done := identityProviderReconciler.Finalize(ctx)
assert.True(t, done)
assert.False(t, test.IsObjectExists(ctx.ClusterAPI.Client, types.NamespacedName{Name: "test"}, &oauthv1.OAuthClient{}))
assert.Equal(t, 0, len(checluster.Finalizers))
}
func TestShouldFindSingleOAuthClient(t *testing.T) {
oauthClient1 := GetOAuthClientSpec("test1", "secret", []string{"https://che-host/oauth/callback"}, nil, nil)
oauthClient2 := GetOAuthClientSpec("test2", "secret", []string{"https://che-host/oauth/callback"}, nil, nil)
checluster := &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eclipse-che",
Namespace: "eclipse-che",
},
Spec: chev2.CheClusterSpec{
Networking: chev2.CheClusterSpecNetworking{
Auth: chev2.Auth{
OAuthClientName: "test1",
},
},
},
}
ctx := test.GetDeployContext(checluster, []runtime.Object{oauthClient1, oauthClient2})
oauthClient, err := GetOAuthClient(ctx)
assert.Nil(t, err)
assert.NotNil(t, oauthClient)
assert.Equal(t, "test1", oauthClient.Name)
}
func TestSyncOAuthClientShouldSyncTokenTimeout(t *testing.T) {
checluster := &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
@ -78,10 +127,11 @@ func TestSyncOAuthClientShouldSyncTokenTimeout(t *testing.T) {
assert.True(t, done)
assert.Nil(t, err)
oauthClients, err := FindAllEclipseCheOAuthClients(ctx)
oauthClient, err := GetOAuthClient(ctx)
assert.Nil(t, err)
assert.Equal(t, int32(10), *oauthClients[0].AccessTokenInactivityTimeoutSeconds)
assert.Equal(t, int32(20), *oauthClients[0].AccessTokenMaxAgeSeconds)
assert.NotNil(t, oauthClient)
assert.Equal(t, int32(10), *oauthClient.AccessTokenInactivityTimeoutSeconds)
assert.Equal(t, int32(20), *oauthClient.AccessTokenMaxAgeSeconds)
}
func TestSyncOAuthClient(t *testing.T) {
@ -113,13 +163,14 @@ func TestSyncOAuthClient(t *testing.T) {
expectedSecret: "secret",
},
{
name: "Sync OAuthClient, generate name and secret",
name: "Sync OAuthClient, generate secret, default name",
cheCluster: &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eclipse-che",
Namespace: "eclipse-che",
},
},
expectedName: "eclipse-che-client",
},
{
name: "Sync OAuthClient, generate secret",
@ -139,7 +190,7 @@ func TestSyncOAuthClient(t *testing.T) {
expectedName: "test",
},
{
name: "Sync OAuthClient, generate name",
name: "Sync OAuthClient, default name",
cheCluster: &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eclipse-che",
@ -154,6 +205,7 @@ func TestSyncOAuthClient(t *testing.T) {
},
},
expectedSecret: "secret",
expectedName: "eclipse-che-client",
},
}
@ -165,21 +217,22 @@ func TestSyncOAuthClient(t *testing.T) {
_, err := syncOAuthClient(ctx)
assert.Nil(t, err)
oauthClients, err := FindAllEclipseCheOAuthClients(ctx)
oauthClient, err := GetOAuthClient(ctx)
assert.Nil(t, err)
assert.Equal(t, 1, len(oauthClients))
assert.NotNil(t, oauthClient)
if testCase.expectedName != "" {
assert.Equal(t, testCase.expectedName, oauthClients[0].Name)
assert.Equal(t, testCase.expectedName, oauthClient.Name)
}
if testCase.expectedSecret != "" {
assert.Equal(t, testCase.expectedSecret, oauthClients[0].Secret)
assert.Equal(t, testCase.expectedSecret, oauthClient.Secret)
}
})
}
}
func TestSyncExistedOAuthClient(t *testing.T) {
oauthClient := GetOAuthClientSpec("test", "secret", []string{}, nil, nil)
oauthClient1 := GetOAuthClientSpec("test", "secret", []string{}, nil, nil)
oauthClient2 := GetOAuthClientSpec("eclipse-che-client", "secret", []string{}, nil, nil)
type testCase struct {
name string
@ -190,7 +243,7 @@ func TestSyncExistedOAuthClient(t *testing.T) {
testCases := []testCase{
{
name: "Sync existed OAuthClient",
name: "Sync existed OAuthClient with the default name",
cheCluster: &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eclipse-che",
@ -198,29 +251,10 @@ func TestSyncExistedOAuthClient(t *testing.T) {
},
},
expectedSecret: "secret",
expectedName: "test",
expectedName: "eclipse-che-client",
},
{
name: "Sync existed OAuthClient, OAuthSecret and OAuthClientName are defined",
cheCluster: &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eclipse-che",
Namespace: "eclipse-che",
},
Spec: chev2.CheClusterSpec{
Networking: chev2.CheClusterSpecNetworking{
Auth: chev2.Auth{
OAuthSecret: "secret",
OAuthClientName: "test",
},
},
},
},
expectedSecret: "secret",
expectedName: "test",
},
{
name: "Sync existed OAuthClient, OAuthClientName is defined",
name: "Sync existed OAuthClient with a custom name",
cheCluster: &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eclipse-che",
@ -238,43 +272,7 @@ func TestSyncExistedOAuthClient(t *testing.T) {
expectedName: "test",
},
{
name: "Sync existed OAuthClient, OAuthSecret is defined",
cheCluster: &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eclipse-che",
Namespace: "eclipse-che",
},
Spec: chev2.CheClusterSpec{
Networking: chev2.CheClusterSpecNetworking{
Auth: chev2.Auth{
OAuthSecret: "secret",
},
},
},
},
expectedSecret: "secret",
expectedName: "test",
},
{
name: "Sync existed OAuthClient, update secret, usecase #1",
cheCluster: &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eclipse-che",
Namespace: "eclipse-che",
},
Spec: chev2.CheClusterSpec{
Networking: chev2.CheClusterSpecNetworking{
Auth: chev2.Auth{
OAuthSecret: "new-secret",
},
},
},
},
expectedSecret: "new-secret",
expectedName: "test",
},
{
name: "Sync existed OAuthClient, update secret, usecase #2",
name: "Sync existed OAuthClient with a custom name, update secret",
cheCluster: &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eclipse-che",
@ -292,24 +290,42 @@ func TestSyncExistedOAuthClient(t *testing.T) {
expectedSecret: "new-secret",
expectedName: "test",
},
{
name: "Sync existed OAuthClient with the default name, update secret",
cheCluster: &chev2.CheCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eclipse-che",
Namespace: "eclipse-che",
},
Spec: chev2.CheClusterSpec{
Networking: chev2.CheClusterSpecNetworking{
Auth: chev2.Auth{
OAuthSecret: "new-secret",
},
},
},
},
expectedSecret: "new-secret",
expectedName: "eclipse-che-client",
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
logf.SetLogger(zap.New(zap.WriteTo(os.Stdout), zap.UseDevMode(true)))
ctx := test.GetDeployContext(testCase.cheCluster, []runtime.Object{oauthClient})
ctx := test.GetDeployContext(testCase.cheCluster, []runtime.Object{oauthClient1, oauthClient2})
_, err := syncOAuthClient(ctx)
assert.Nil(t, err)
oauthClients, err := FindAllEclipseCheOAuthClients(ctx)
oauthClient, err := GetOAuthClient(ctx)
assert.Nil(t, err)
assert.Equal(t, 1, len(oauthClients))
assert.NotNil(t, oauthClient)
if testCase.expectedName != "" {
assert.Equal(t, testCase.expectedName, oauthClients[0].Name)
assert.Equal(t, testCase.expectedName, oauthClient.Name)
}
if testCase.expectedSecret != "" {
assert.Equal(t, testCase.expectedSecret, oauthClients[0].Secret)
assert.Equal(t, testCase.expectedSecret, oauthClient.Secret)
}
})
}

40
pkg/deploy/identity-provider/identity_provider_util.go
View File

@ -12,15 +12,11 @@
package identityprovider
import (
"context"
"fmt"
"github.com/eclipse-che/che-operator/pkg/common/chetypes"
"github.com/eclipse-che/che-operator/pkg/common/constants"
"github.com/eclipse-che/che-operator/pkg/deploy"
oauth "github.com/openshift/api/oauth/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"sigs.k8s.io/controller-runtime/pkg/client"
)
func GetOAuthClientSpec(
@ -29,6 +25,7 @@ func GetOAuthClientSpec(
redirectURIs []string,
accessTokenInactivityTimeoutSeconds *int32,
accessTokenMaxAgeSeconds *int32) *oauth.OAuthClient {
return &oauth.OAuthClient{
TypeMeta: metav1.TypeMeta{
Kind: "OAuthClient",
@ -47,31 +44,22 @@ func GetOAuthClientSpec(
}
}
func FindOAuthClient(ctx *chetypes.DeployContext) (*oauth.OAuthClient, error) {
oauthClients, err := FindAllEclipseCheOAuthClients(ctx)
if err != nil {
func GetOAuthClient(ctx *chetypes.DeployContext) (*oauth.OAuthClient, error) {
oAuthClientName := GetOAuthClientName(ctx)
oauthClient := &oauth.OAuthClient{}
exists, err := deploy.GetClusterObject(ctx, oAuthClientName, oauthClient)
if !exists {
return nil, err
}
switch len(oauthClients) {
case 0:
return nil, nil
case 1:
return &oauthClients[0], nil
default:
return nil, fmt.Errorf("more than one OAuthClient found with '%s:%s' labels", constants.KubernetesPartOfLabelKey, constants.CheEclipseOrg)
}
return oauthClient, nil
}
func FindAllEclipseCheOAuthClients(ctx *chetypes.DeployContext) ([]oauth.OAuthClient, error) {
oauthClients := &oauth.OAuthClientList{}
listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(map[string]string{constants.KubernetesPartOfLabelKey: constants.CheEclipseOrg})}
if err := ctx.ClusterAPI.Client.List(
context.TODO(),
oauthClients,
listOptions); err != nil {
return nil, err
func GetOAuthClientName(ctx *chetypes.DeployContext) string {
if ctx.CheCluster.Spec.Networking.Auth.OAuthClientName != "" {
return ctx.CheCluster.Spec.Networking.Auth.OAuthClientName
}
return oauthClients.Items, nil
return ctx.CheCluster.Namespace + "-client"
}