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.

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

Add OAuth to external endpoints #140

Merged
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
07a72df
Add OAuth to external endpoints
ruivieira Nov 2, 2023
8239336
Change ServiceAccount. Remove unnecessary secrets.
ruivieira Nov 6, 2023
8798760
Remove scope
ruivieira Nov 7, 2023
e53be78
Add ClusterRoleBindings creation
ruivieira Nov 7, 2023
e85deda
Move OAuth proxy image reference to params.env
ruivieira Nov 8, 2023
a8a9202
Merge remote-tracking branch 'upstream/main' into issue-135-oauth-proxy
ruivieira Nov 14, 2023
5d7bda0
Fix route test
ruivieira Nov 14, 2023
9fb4980
Refactor deployment tests
ruivieira Nov 14, 2023
36dbe4c
Merge remote-tracking branch 'upstream/main' into issue-135-oauth-proxy
ruivieira Nov 17, 2023
2a814ee
Refactor suite_test.go file
ruivieira Nov 18, 2023
703614b
Update dependencies in go.mod file
ruivieira Nov 18, 2023
783ff07
Add time import to trustyaiservice_controller.go
ruivieira Nov 18, 2023
c279b99
Add container images to deployment spec
ruivieira Nov 19, 2023
d8f418b
Add OAuth-proxy image retrieval from ConfigMap
ruivieira Nov 19, 2023
1f22cb5
Refactor deployment_test.go file
ruivieira Nov 19, 2023
075d558
Fix OAuth volume generation
ruivieira Nov 19, 2023
ab704fd
Add WaitFor function to suite_test.go
ruivieira Nov 19, 2023
b71974f
Add test for custom images
ruivieira Nov 19, 2023
94a4580
Add ConfigMap tests
ruivieira Nov 19, 2023
8d3a6aa
Add fake client builder for unit tests
ruivieira Nov 19, 2023
b048f0b
Fix ConfigMap deletion in tests
ruivieira Nov 19, 2023
b808374
Add test case for default values in ConfigMap
ruivieira Nov 19, 2023
8bfbaca
Add ConfigMap test for deploying with wrong keys
ruivieira Nov 19, 2023
41266f7
Refactor route creation in route_test.go
ruivieira Nov 19, 2023
3e8ed8b
Refactor status and condition tests
ruivieira Nov 19, 2023
e5b2547
Refactor storage_test.go to improve PVC
ruivieira Nov 19, 2023
c929d3e
Merge remote-tracking branch 'upstream/main' into issue-135-oauth-proxy
ruivieira Nov 27, 2023
68dee4f
Merge remote-tracking branch 'upstream/main' into issue-135-oauth-proxy
ruivieira Nov 28, 2023
540d92c
Formatting of rbac/role.yaml
ruivieira Nov 28, 2023
7f37419
Formatting of base/kustomization.yaml
ruivieira Nov 28, 2023
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
9 changes: 8 additions & 1 deletion config/base/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,11 @@ vars:
name: config
apiVersion: v1
fieldref:
fieldpath: data.trustyaiOperatorImage
fieldpath: data.trustyaiOperatorImage
- name: oauthProxyImage
objref:
kind: ConfigMap
name: config
apiVersion: v1
fieldref:
fieldpath: data.oauthProxyImage
3 changes: 2 additions & 1 deletion config/base/params.env
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
trustyaiServiceImage=quay.io/trustyai/trustyai-service:latest
trustyaiOperatorImage=quay.io/trustyai/trustyai-service-operator:latest
trustyaiOperatorImage=quay.io/trustyai/trustyai-service-operator:latest
oauthProxyImage=registry.redhat.io/openshift4/ose-oauth-proxy:latest
11 changes: 11 additions & 0 deletions config/rbac/auth-delegator.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: manager-auth-delegator
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: system:auth-delegator
subjects:
- kind: ServiceAccount
name: controller-manager
34 changes: 34 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,29 @@ rules:
- patch
- update
- watch
- apiGroups:
- ""
resources:
- secrets
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- ""
resources:
- serviceaccounts
verbs:
- create
- delete
- get
- list
- update
- watch
- apiGroups:
- apps
resources:
Expand Down Expand Up @@ -103,6 +126,17 @@ rules:
- create
- list
- watch
- apiGroups:
- rbac.authorization.k8s.io
resources:
- clusterrolebindings
verbs:
- create
- delete
- get
- list
- update
- watch
- apiGroups:
- route.openshift.io
resources:
Expand Down
46 changes: 46 additions & 0 deletions controllers/config_maps.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package controllers

import (
"context"
"fmt"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
)

// getImageFromConfigMap gets a custom image value from a ConfigMap in the operator's namespace
func (r *TrustyAIServiceReconciler) getImageFromConfigMap(ctx context.Context, key string, defaultImage string) (string, error) {
if r.Namespace != "" {
// Define the key for the ConfigMap
configMapKey := types.NamespacedName{
Namespace: r.Namespace,
Name: imageConfigMap,
}

// Create an empty ConfigMap object
var cm corev1.ConfigMap

// Try to get the ConfigMap
if err := r.Get(ctx, configMapKey, &cm); err != nil {
if errors.IsNotFound(err) {
// ConfigMap not found, fallback to default values
return defaultImage, nil
}
// Other error occurred when trying to fetch the ConfigMap
return defaultImage, fmt.Errorf("error reading configmap %s", configMapKey)
}

// ConfigMap is found, extract the image and tag
image, ok := cm.Data[key]

if !ok {
// One or both of the keys are not present in the ConfigMap, return error
return defaultImage, fmt.Errorf("configmap %s does not contain necessary keys", configMapKey)
}

// Return the image and tag
return image, nil
} else {
return defaultImage, nil
}
}
147 changes: 147 additions & 0 deletions controllers/config_maps_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
package controllers

import (
"context"
"fmt"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

var _ = Describe("ConfigMap tests", func() {

BeforeEach(func() {
recorder = record.NewFakeRecorder(10)
k8sClient = fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
reconciler = &TrustyAIServiceReconciler{
Client: k8sClient,
Scheme: scheme.Scheme,
EventRecorder: recorder,
Namespace: operatorNamespace,
}
ctx = context.Background()
})

AfterEach(func() {
// Attempt to delete the ConfigMap
configMap := &corev1.ConfigMap{}
err := k8sClient.Get(ctx, types.NamespacedName{
Namespace: operatorNamespace,
Name: imageConfigMap,
}, configMap)

// If the ConfigMap exists, delete it
if err == nil {
Expect(k8sClient.Delete(ctx, configMap)).To(Succeed())
} else if !apierrors.IsNotFound(err) {
Fail(fmt.Sprintf("Unexpected error while getting ConfigMap: %s", err))
}
})

Context("When deploying a ConfigMap to the operator's namespace", func() {

It("Should get back the correct values", func() {

serviceImage := "custom-service-image:foo"
oauthImage := "custom-oauth-proxy:bar"

WaitFor(func() error {
configMap := createConfigMap(operatorNamespace, oauthImage, serviceImage)
return k8sClient.Create(ctx, configMap)
}, "failed to create ConfigMap")

var actualOAuthImage string
var actualServiceImage string

WaitFor(func() error {
var err error
actualOAuthImage, err = reconciler.getImageFromConfigMap(ctx, configMapOAuthProxyImageKey, defaultOAuthProxyImage)
return err
}, "failed to get oauth image from ConfigMap")

WaitFor(func() error {
var err error
actualServiceImage, err = reconciler.getImageFromConfigMap(ctx, configMapServiceImageKey, defaultImage)
return err
}, "failed to get service image from ConfigMap")

Expect(actualOAuthImage).Should(Equal(oauthImage))
Expect(actualServiceImage).Should(Equal(serviceImage))
})
})

Context("When no ConfigMap in the operator's namespace", func() {

It("Should get back the default values", func() {

var actualOAuthImage string
var actualServiceImage string

WaitFor(func() error {
var err error
actualOAuthImage, err = reconciler.getImageFromConfigMap(ctx, configMapOAuthProxyImageKey, defaultOAuthProxyImage)
return err
}, "failed to get oauth image from ConfigMap")

WaitFor(func() error {
var err error
actualServiceImage, err = reconciler.getImageFromConfigMap(ctx, configMapServiceImageKey, defaultImage)
return err
}, "failed to get service image from ConfigMap")

Expect(actualOAuthImage).Should(Equal(defaultOAuthProxyImage))
Expect(actualServiceImage).Should(Equal(defaultImage))
})
})

Context("When deploying a ConfigMap to the operator's namespace with the wrong keys", func() {

It("Should get back the default values", func() {

serviceImage := "custom-service-image:foo"
oauthImage := "custom-oauth-proxy:bar"

WaitFor(func() error {
configMap := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: imageConfigMap,
Namespace: operatorNamespace,
},
Data: map[string]string{
"foo-oauth-image": oauthImage,
"foo-image": serviceImage,
},
}
return k8sClient.Create(ctx, configMap)
}, "failed to create ConfigMap")

var actualOAuthImage string
var actualServiceImage string

configMapPath := operatorNamespace + "/" + imageConfigMap

Eventually(func() error {
var err error
actualOAuthImage, err = reconciler.getImageFromConfigMap(ctx, configMapOAuthProxyImageKey, defaultOAuthProxyImage)
return err
}, defaultTimeout, defaultPolling).Should(MatchError(fmt.Sprintf("configmap %s does not contain necessary keys", configMapPath)), "failed to get oauth image from ConfigMap")

Eventually(func() error {
var err error
actualServiceImage, err = reconciler.getImageFromConfigMap(ctx, configMapServiceImageKey, defaultImage)
return err
}, defaultTimeout, defaultPolling).Should(MatchError(fmt.Sprintf("configmap %s does not contain necessary keys", configMapPath)), "failed to get oauth image from ConfigMap")

Expect(actualOAuthImage).Should(Equal(defaultOAuthProxyImage))
Expect(actualServiceImage).Should(Equal(defaultImage))
})
})

})
15 changes: 15 additions & 0 deletions controllers/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,21 @@ const (
defaultRequeueDelay = time.Minute
)

// Configuration constants
const (
imageConfigMap = "trustyai-service-operator-config"
configMapOAuthProxyImageKey = "oauthProxyImage"
configMapServiceImageKey = "trustyaiServiceImage"
)

// OAuth constants
const (
OAuthServicePort = 443
OAuthName = "oauth-proxy"
OAuthServicePortName = "oauth-proxy"
defaultOAuthProxyImage = "registry.redhat.io/openshift4/ose-oauth-proxy:latest"
)

// Status types
const (
StatusTypeInferenceServicesPresent = "InferenceServicesPresent"
Expand Down
46 changes: 32 additions & 14 deletions controllers/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package controllers

import (
"context"
"strconv"

trustyaiopendatahubiov1alpha1 "github.com/trustyai-explainability/trustyai-service-operator/api/v1alpha1"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand All @@ -10,12 +12,13 @@ import (
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/log"
"strconv"
)

// createDeploymentObject returns a Deployment for the TrustyAI Service instance
func (r *TrustyAIServiceReconciler) createDeploymentObject(ctx context.Context, cr *trustyaiopendatahubiov1alpha1.TrustyAIService, image string) *appsv1.Deployment {
labels := getCommonLabels(cr.Name)
pvcName := generatePVCName(cr)
serviceAccountName := generateServiceAccountName(cr)

replicas := int32(1)
if cr.Spec.Replicas == nil {
Expand All @@ -29,6 +32,15 @@ func (r *TrustyAIServiceReconciler) createDeploymentObject(ctx context.Context,
batchSize = *cr.Spec.Metrics.BatchSize
}

// Create the OAuth-Proxy container spec

// Get OAuth-proxy image from ConfigMap
oauthProxyImage, err := r.getImageFromConfigMap(ctx, configMapOAuthProxyImageKey, defaultOAuthProxyImage)
if err != nil {
log.FromContext(ctx).Error(err, "Error getting OAuth image from ConfigMap. Using the default image value of "+defaultOAuthProxyImage)
}
oauthProxyContainer := generateOAuthProxyContainer(cr, oauthProxyImage)

containers := []corev1.Container{
{
Name: containerName,
Expand Down Expand Up @@ -67,8 +79,23 @@ func (r *TrustyAIServiceReconciler) createDeploymentObject(ctx context.Context,
},
},
},
oauthProxyContainer,
}

volume := corev1.Volume{

Name: volumeMountName,
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: pvcName,
ReadOnly: false,
},
},
}
volumes := generateOAuthVolumes(cr, OAuthConfig{ProxyImage: defaultOAuthProxyImage})

volumes = append(volumes, volume)

deployment := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: cr.Name,
Expand All @@ -90,18 +117,9 @@ func (r *TrustyAIServiceReconciler) createDeploymentObject(ctx context.Context,
},
},
Spec: corev1.PodSpec{
Containers: containers,
Volumes: []corev1.Volume{
{
Name: volumeMountName,
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: pvcName,
ReadOnly: false,
},
},
},
},
ServiceAccountName: serviceAccountName,
Containers: containers,
Volumes: volumes,
},
},
},
Expand Down Expand Up @@ -148,7 +166,7 @@ func (r *TrustyAIServiceReconciler) ensureDeployment(ctx context.Context, instan
// Get image and tag from ConfigMap
// If there's a ConfigMap with custom images, it is only applied when the operator is first deployed
// Changing (or creating) the ConfigMap after the operator is deployed will not have any effect
image, err := r.getImageFromConfigMap(ctx)
image, err := r.getImageFromConfigMap(ctx, configMapServiceImageKey, defaultImage)
if err != nil {
return err
}
Expand Down
Loading
Loading