Skip to content

Commit

Permalink
ARO-4639 update the operator master deployment to support workload id…
Browse files Browse the repository at this point in the history
…entity (#3776)

* update the operator master deployment to support workload identity

This causes the spec for the operator master deployment to mount the
service account token as a volume, and maps the path to the environment
variable expected by Azure to support workload identities

* remove unused ExpectError value from test struct

* mount the token secret as a directory, not a file
  • Loading branch information
yithian authored Sep 18, 2024
1 parent 813de36 commit bd47ae7
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 6 deletions.
14 changes: 12 additions & 2 deletions pkg/operator/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"embed"
"errors"
"fmt"
"path/filepath"
"strings"
"text/template"
"time"
Expand Down Expand Up @@ -107,6 +108,8 @@ type deploymentData struct {
Version string
IsLocalDevelopment bool
SupportsPodSecurityAdmission bool
UsesWorkloadIdentity bool
TokenVolumeMountPath string
}

func (o *operator) SetForceReconcile(ctx context.Context, enable bool) error {
Expand Down Expand Up @@ -177,12 +180,19 @@ func (o *operator) createDeploymentData(ctx context.Context) (deploymentData, er
return deploymentData{}, err
}

return deploymentData{
data := deploymentData{
IsLocalDevelopment: o.env.IsLocalDevelopmentMode(),
Image: image,
SupportsPodSecurityAdmission: usePodSecurityAdmission,
Version: version,
}, nil
}

if o.oc.UsesWorkloadIdentity() {
data.UsesWorkloadIdentity = o.oc.UsesWorkloadIdentity()
data.TokenVolumeMountPath = filepath.Dir(pkgoperator.OperatorTokenFile)
}

return data, nil
}

func (o *operator) createObjects(ctx context.Context) ([]kruntime.Object, error) {
Expand Down
87 changes: 84 additions & 3 deletions pkg/operator/deploy/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package deploy

import (
"context"
"path/filepath"
"reflect"
"testing"

Expand All @@ -16,8 +17,10 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kruntime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes/fake"
ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/yaml"

"github.com/Azure/ARO-RP/pkg/api"
pkgoperator "github.com/Azure/ARO-RP/pkg/operator"
Expand Down Expand Up @@ -205,6 +208,39 @@ func TestCreateDeploymentData(t *testing.T) {
SupportsPodSecurityAdmission: true,
},
},
{
name: "workload identity detected",
mock: func(env *mock_env.MockInterface, oc *api.OpenShiftCluster) {
env.EXPECT().
AROOperatorImage().
Return(operatorImageWithTag)
// set so that UsesWorkloadIdentity() returns true
oc.Properties.PlatformWorkloadIdentityProfile = &api.PlatformWorkloadIdentityProfile{}
},
clusterVersion: "4.10.0",
expected: deploymentData{
Image: operatorImageWithTag,
Version: operatorImageTag,
UsesWorkloadIdentity: true,
TokenVolumeMountPath: filepath.Dir(pkgoperator.OperatorTokenFile),
},
},
{
name: "service principal detected",
mock: func(env *mock_env.MockInterface, oc *api.OpenShiftCluster) {
env.EXPECT().
AROOperatorImage().
Return(operatorImageWithTag)
// set so that UsesWorkloadIdentity() returns false
oc.Properties.ServicePrincipalProfile = &api.ServicePrincipalProfile{}
},
clusterVersion: "4.10.0",
expected: deploymentData{
Image: operatorImageWithTag,
Version: operatorImageTag,
UsesWorkloadIdentity: false,
},
},
} {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()
Expand Down Expand Up @@ -591,7 +627,6 @@ func TestGenerateOperatorIdentitySecret(t *testing.T) {
Name string
Operator *operator
ExpectedSecret *corev1.Secret
ExpectError bool
}{
{
Name: "valid cluster operator",
Expand Down Expand Up @@ -638,8 +673,8 @@ func TestGenerateOperatorIdentitySecret(t *testing.T) {
for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
actualSecret, err := test.Operator.generateOperatorIdentitySecret()
if test.ExpectError == (err == nil) {
t.Errorf("generateOperatorIdentitySecret() %s: ExpectError: %t, actual error: %s\n", test.Name, test.ExpectError, err)
if err != nil {
t.Errorf("generateOperatorIdentitySecret() %s: unexpected error: %s\n", test.Name, err)
}

if !reflect.DeepEqual(actualSecret, test.ExpectedSecret) {
Expand All @@ -648,3 +683,49 @@ func TestGenerateOperatorIdentitySecret(t *testing.T) {
})
}
}

func TestTemplateManifests(t *testing.T) {
tests := []struct {
Name string
DeploymentData deploymentData
}{
{
Name: "service principal data",
DeploymentData: deploymentData{
Image: "someImage",
Version: "someVersion",
IsLocalDevelopment: false,
SupportsPodSecurityAdmission: false,
UsesWorkloadIdentity: false,
},
},
{
Name: "workload identity data",
DeploymentData: deploymentData{
Image: "someImage",
Version: "someVersion",
IsLocalDevelopment: false,
SupportsPodSecurityAdmission: false,
UsesWorkloadIdentity: true,
},
},
}

for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
actualBytes, err := templateManifests(test.DeploymentData)
if err != nil {
t.Errorf("templateManifests() %s: unexpected error: %s\n", test.Name, err)
}

for _, fileBytes := range actualBytes {
var resource *kruntime.Object
err := yaml.Unmarshal(fileBytes, resource)

if err != nil {
t.Errorf("templateManifests() %s: unexpected error: %s\n", test.Name, err)
}
}
})
}
}
23 changes: 22 additions & 1 deletion pkg/operator/deploy/staticresources/master/deployment.yaml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,16 @@ spec:
secretKeyRef:
name: azure-cloud-credentials
key: azure_client_id
{{ if .UsesWorkloadIdentity }}
- name: AZURE_FEDERATED_TOKEN_FILE
value: "{{ .TokenVolumeMountPath }}"
{{ else }}
- name: AZURE_CLIENT_SECRET
valueFrom:
secretKeyRef:
name: azure-cloud-credentials
key: azure_client_secret
{{ end }}
- name: AZURE_TENANT_ID
valueFrom:
secretKeyRef:
Expand All @@ -63,6 +68,12 @@ spec:
- ALL
runAsNonRoot: true
{{ end }}
{{ if .UsesWorkloadIdentity }}
volumeMounts:
- mountPath: "{{ .TokenVolumeMountPath }}"
name: bound-sa-token
readOnly: true
{{ end }}
nodeSelector:
node-role.kubernetes.io/master: ""
{{ if .SupportsPodSecurityAdmission }}
Expand All @@ -79,4 +90,14 @@ spec:
operator: Exists
- effect: NoSchedule
operator: Exists

{{ if .UsesWorkloadIdentity }}
volumes:
- name: bound-sa-token
projected:
defaultMode: 420
sources:
- serviceAccountToken:
audience: openshift
expirationSeconds: 3600
path: token
{{ end }}

0 comments on commit bd47ae7

Please sign in to comment.