Skip to content

Commit

Permalink
Allow API to manage AD Users and Reader AD Users (#677)
Browse files Browse the repository at this point in the history
* Allow API to manage AD Users and Reader AD Users

* Update swagger

* Add test for Getting application, ad users is set

* Update operator

* Update types to imply AdUsers is always set

* update operator
  • Loading branch information
Richard87 authored Sep 25, 2024
1 parent cd1c04e commit 9cc57b2
Show file tree
Hide file tree
Showing 8 changed files with 120 additions and 16 deletions.
39 changes: 28 additions & 11 deletions api/applications/applications_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ import (
"github.com/equinor/radix-operator/pkg/apis/radixvalidators"
commontest "github.com/equinor/radix-operator/pkg/apis/test"
builders "github.com/equinor/radix-operator/pkg/apis/utils"
"github.com/equinor/radix-operator/pkg/client/clientset/versioned/fake"
radixfake "github.com/equinor/radix-operator/pkg/client/clientset/versioned/fake"
"github.com/google/uuid"
kedafake "github.com/kedacore/keda/v2/pkg/generated/clientset/versioned/fake"
prometheusfake "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned/fake"
"github.com/stretchr/testify/assert"
Expand All @@ -49,7 +50,7 @@ const (
subscriptionId = "12347718-c8f8-4995-bfbb-02655ff1f89c"
)

func setupTest(t *testing.T, requireAppConfigurationItem, requireAppADGroups bool) (*commontest.Utils, *controllertest.Utils, *kubefake.Clientset, *fake.Clientset, *kedafake.Clientset, *prometheusfake.Clientset, *secretproviderfake.Clientset, *certfake.Clientset) {
func setupTest(t *testing.T, requireAppConfigurationItem, requireAppADGroups bool) (*commontest.Utils, *controllertest.Utils, *kubefake.Clientset, *radixfake.Clientset, *kedafake.Clientset, *prometheusfake.Clientset, *secretproviderfake.Clientset, *certfake.Clientset) {
return setupTestWithFactory(t, newTestApplicationHandlerFactory(
ApplicationHandlerConfig{RequireAppConfigurationItem: requireAppConfigurationItem, RequireAppADGroups: requireAppADGroups},
func(ctx context.Context, kubeClient kubernetes.Interface, namespace string, configMapName string) (bool, error) {
Expand All @@ -58,10 +59,10 @@ func setupTest(t *testing.T, requireAppConfigurationItem, requireAppADGroups boo
))
}

func setupTestWithFactory(t *testing.T, handlerFactory ApplicationHandlerFactory) (*commontest.Utils, *controllertest.Utils, *kubefake.Clientset, *fake.Clientset, *kedafake.Clientset, *prometheusfake.Clientset, *secretproviderfake.Clientset, *certfake.Clientset) {
func setupTestWithFactory(t *testing.T, handlerFactory ApplicationHandlerFactory) (*commontest.Utils, *controllertest.Utils, *kubefake.Clientset, *radixfake.Clientset, *kedafake.Clientset, *prometheusfake.Clientset, *secretproviderfake.Clientset, *certfake.Clientset) {
// Setup
kubeclient := kubefake.NewSimpleClientset()
radixclient := fake.NewSimpleClientset()
radixclient := radixfake.NewSimpleClientset()
kedaClient := kedafake.NewSimpleClientset()
prometheusclient := prometheusfake.NewSimpleClientset()
secretproviderclient := secretproviderfake.NewSimpleClientset()
Expand Down Expand Up @@ -809,12 +810,17 @@ func TestGetApplication_AllFieldsAreSet(t *testing.T) {
// Setup
_, controllerTestUtils, _, _, _, _, _, _ := setupTest(t, true, true)

adGroups, adUsers := []string{uuid.New().String()}, []string{uuid.New().String()}
readerAdGroups, readerAdUsers := []string{uuid.New().String()}, []string{uuid.New().String()}
parameters := buildApplicationRegistrationRequest(
anApplicationRegistration().
WithName("any-name").
WithRepository("https://github.com/Equinor/any-repo").
WithSharedSecret("Any secret").
WithAdGroups([]string{"a6a3b81b-34gd-sfsf-saf2-7986371ea35f"}).
WithAdGroups(adGroups).
WithAdUsers(adUsers).
WithReaderAdGroups(readerAdGroups).
WithReaderAdUsers(readerAdUsers).
WithConfigBranch("abranch").
WithRadixConfigFullName("a/custom-radixconfig.yaml").
WithConfigurationItem("ci").
Expand All @@ -835,7 +841,10 @@ func TestGetApplication_AllFieldsAreSet(t *testing.T) {

assert.Equal(t, "https://github.com/Equinor/any-repo", application.Registration.Repository)
assert.Equal(t, "Any secret", application.Registration.SharedSecret)
assert.Equal(t, []string{"a6a3b81b-34gd-sfsf-saf2-7986371ea35f"}, application.Registration.AdGroups)
assert.Equal(t, adGroups, application.Registration.AdGroups)
assert.Equal(t, adUsers, application.Registration.AdUsers)
assert.Equal(t, readerAdGroups, application.Registration.ReaderAdGroups)
assert.Equal(t, readerAdUsers, application.Registration.ReaderAdUsers)
assert.Equal(t, "[email protected]", application.Registration.Creator)
assert.Equal(t, "abranch", application.Registration.ConfigBranch)
assert.Equal(t, "a/custom-radixconfig.yaml", application.Registration.RadixConfigFullName)
Expand Down Expand Up @@ -1157,8 +1166,10 @@ func TestModifyApplication_AbleToSetField(t *testing.T) {
WithName("any-name").
WithRepository("https://github.com/Equinor/a-repo").
WithSharedSecret("").
WithAdGroups([]string{"a5dfa635-dc00-4a28-9ad9-9e7f1e56919d"}).
WithReaderAdGroups([]string{"d5df55c1-78b7-4330-9d2c-f1b1aa5584ca"}).
WithAdGroups([]string{uuid.New().String()}).
WithAdUsers([]string{uuid.New().String()}).
WithReaderAdGroups([]string{uuid.New().String()}).
WithReaderAdUsers([]string{uuid.New().String()}).
WithOwner("[email protected]").
WithWBS("T.O123A.AZ.45678").
WithConfigBranch("main1").
Expand All @@ -1167,10 +1178,12 @@ func TestModifyApplication_AbleToSetField(t *testing.T) {
<-responseChannel

// Test
anyNewAdGroup := []string{"98765432-dc00-4a28-9ad9-9e7f1e56919d"}
anyNewAdGroup := []string{uuid.New().String()}
anyNewAdUser := []string{uuid.New().String()}
patchRequest := applicationModels.ApplicationRegistrationPatchRequest{
ApplicationRegistrationPatch: &applicationModels.ApplicationRegistrationPatch{
AdGroups: &anyNewAdGroup,
AdUsers: &anyNewAdUser,
},
}

Expand All @@ -1185,16 +1198,19 @@ func TestModifyApplication_AbleToSetField(t *testing.T) {
err := controllertest.GetResponseBody(response, &application)
require.NoError(t, err)
assert.Equal(t, anyNewAdGroup, application.Registration.AdGroups)
assert.Equal(t, anyNewAdUser, application.Registration.AdUsers)
assert.Equal(t, "[email protected]", application.Registration.Owner)
assert.Equal(t, "T.O123A.AZ.45678", application.Registration.WBS)
assert.Equal(t, "main1", application.Registration.ConfigBranch)
assert.Equal(t, "ci-initial", application.Registration.ConfigurationItem)

// Test
anyNewReaderAdGroup := []string{"44643b96-0f6d-4bdc-af2c-a4f596d821eb"}
anyNewReaderAdGroup := []string{uuid.New().String()}
anyNewReaderAdUser := []string{uuid.New().String()}
patchRequest = applicationModels.ApplicationRegistrationPatchRequest{
ApplicationRegistrationPatch: &applicationModels.ApplicationRegistrationPatch{
ReaderAdGroups: &anyNewReaderAdGroup,
ReaderAdUsers: &anyNewReaderAdUser,
},
}

Expand All @@ -1209,6 +1225,7 @@ func TestModifyApplication_AbleToSetField(t *testing.T) {
err = controllertest.GetResponseBody(response, &application)
require.NoError(t, err)
assert.Equal(t, anyNewReaderAdGroup, application.Registration.ReaderAdGroups)
assert.Equal(t, anyNewReaderAdUser, application.Registration.ReaderAdUsers)

// Test
anyNewOwner := "[email protected]"
Expand Down Expand Up @@ -1924,7 +1941,7 @@ func createRadixJob(commonTestUtils *commontest.Utils, appName, jobName string,
return err
}

func getJobsInNamespace(radixclient *fake.Clientset, appNamespace string) ([]v1.RadixJob, error) {
func getJobsInNamespace(radixclient *radixfake.Clientset, appNamespace string) ([]v1.RadixJob, error) {
jobs, err := radixclient.RadixV1().RadixJobs(appNamespace).List(context.Background(), metav1.ListOptions{})
if err != nil {
return nil, err
Expand Down
10 changes: 10 additions & 0 deletions api/applications/applications_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,11 +283,21 @@ func (ah *ApplicationHandler) ModifyRegistrationDetails(ctx context.Context, app
payload = append(payload, patch{Op: "replace", Path: "/spec/adGroups", Value: *patchRequest.AdGroups})
runUpdate = true
}
if patchRequest.AdUsers != nil && !radixutils.ArrayEqualElements(currentRegistration.Spec.AdUsers, *patchRequest.AdUsers) {
updatedRegistration.Spec.AdUsers = *patchRequest.AdUsers
payload = append(payload, patch{Op: "replace", Path: "/spec/adUsers", Value: *patchRequest.AdUsers})
runUpdate = true
}
if patchRequest.ReaderAdGroups != nil && !radixutils.ArrayEqualElements(currentRegistration.Spec.ReaderAdGroups, *patchRequest.ReaderAdGroups) {
updatedRegistration.Spec.ReaderAdGroups = *patchRequest.ReaderAdGroups
payload = append(payload, patch{Op: "replace", Path: "/spec/readerAdGroups", Value: *patchRequest.ReaderAdGroups})
runUpdate = true
}
if patchRequest.ReaderAdUsers != nil && !radixutils.ArrayEqualElements(currentRegistration.Spec.ReaderAdUsers, *patchRequest.ReaderAdUsers) {
updatedRegistration.Spec.ReaderAdUsers = *patchRequest.ReaderAdUsers
payload = append(payload, patch{Op: "replace", Path: "/spec/readerAdUsers", Value: *patchRequest.ReaderAdUsers})
runUpdate = true
}

if patchRequest.Owner != nil && *patchRequest.Owner != "" {
updatedRegistration.Spec.Owner = *patchRequest.Owner
Expand Down
12 changes: 11 additions & 1 deletion api/applications/models/application_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,21 @@ type ApplicationRegistration struct {
// required: true
AdGroups []string `json:"adGroups"`

// AdUsers the users/service-principals that should be able to access the application
//
// required: true
AdUsers []string `json:"adUsers"`

// ReaderAdGroups the groups that should be able to read the application
//
// required: false
// required: true
ReaderAdGroups []string `json:"readerAdGroups"`

// ReaderAdUsers the users/service-principals that should be able to read the application
//
// required: true
ReaderAdUsers []string `json:"readerAdUsers"`

// Owner of the application (email). Can be a single person or a shared group email
//
// required: true
Expand Down
22 changes: 22 additions & 0 deletions api/applications/models/application_registration_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ type ApplicationRegistrationBuilder interface {
WithRepository(string) ApplicationRegistrationBuilder
WithSharedSecret(string) ApplicationRegistrationBuilder
WithAdGroups([]string) ApplicationRegistrationBuilder
WithAdUsers([]string) ApplicationRegistrationBuilder
WithReaderAdGroups([]string) ApplicationRegistrationBuilder
WithReaderAdUsers([]string) ApplicationRegistrationBuilder
WithCloneURL(string) ApplicationRegistrationBuilder
WithWBS(string) ApplicationRegistrationBuilder
WithConfigBranch(string) ApplicationRegistrationBuilder
Expand All @@ -38,14 +40,18 @@ type applicationBuilder struct {
configBranch string
configurationItem string
radixConfigFullName string
adUsers []string
readerAdUsers []string
}

func (rb *applicationBuilder) WithAppRegistration(appRegistration *ApplicationRegistration) ApplicationRegistrationBuilder {
rb.WithName(appRegistration.Name)
rb.WithRepository(appRegistration.Repository)
rb.WithSharedSecret(appRegistration.SharedSecret)
rb.WithAdGroups(appRegistration.AdGroups)
rb.WithAdUsers(appRegistration.AdUsers)
rb.WithReaderAdGroups(appRegistration.ReaderAdGroups)
rb.WithReaderAdUsers(appRegistration.ReaderAdUsers)
rb.WithOwner(appRegistration.Owner)
rb.WithWBS(appRegistration.WBS)
rb.WithConfigBranch(appRegistration.ConfigBranch)
Expand All @@ -59,7 +65,9 @@ func (rb *applicationBuilder) WithRadixRegistration(radixRegistration *v1.RadixR
rb.WithCloneURL(radixRegistration.Spec.CloneURL)
rb.WithSharedSecret(radixRegistration.Spec.SharedSecret)
rb.WithAdGroups(radixRegistration.Spec.AdGroups)
rb.WithAdUsers(radixRegistration.Spec.AdUsers)
rb.WithReaderAdGroups(radixRegistration.Spec.ReaderAdGroups)
rb.WithReaderAdUsers(radixRegistration.Spec.ReaderAdUsers)
rb.WithOwner(radixRegistration.Spec.Owner)
rb.WithCreator(radixRegistration.Spec.Creator)
rb.WithWBS(radixRegistration.Spec.WBS)
Expand Down Expand Up @@ -106,11 +114,21 @@ func (rb *applicationBuilder) WithAdGroups(adGroups []string) ApplicationRegistr
return rb
}

func (rb *applicationBuilder) WithAdUsers(adUsers []string) ApplicationRegistrationBuilder {
rb.adUsers = adUsers
return rb
}

func (rb *applicationBuilder) WithReaderAdGroups(readerAdGroups []string) ApplicationRegistrationBuilder {
rb.readerAdGroups = readerAdGroups
return rb
}

func (rb *applicationBuilder) WithReaderAdUsers(readerAdUsers []string) ApplicationRegistrationBuilder {
rb.readerAdUsers = readerAdUsers
return rb
}

func (rb *applicationBuilder) WithWBS(wbs string) ApplicationRegistrationBuilder {
rb.wbs = wbs
return rb
Expand Down Expand Up @@ -142,7 +160,9 @@ func (rb *applicationBuilder) Build() ApplicationRegistration {
Repository: repository,
SharedSecret: rb.sharedSecret,
AdGroups: rb.adGroups,
AdUsers: rb.adUsers,
ReaderAdGroups: rb.readerAdGroups,
ReaderAdUsers: rb.readerAdUsers,
Owner: rb.owner,
Creator: rb.creator,
WBS: rb.wbs,
Expand All @@ -160,7 +180,9 @@ func (rb *applicationBuilder) BuildRR() (*v1.RadixRegistration, error) {
WithRepository(rb.repository).
WithSharedSecret(rb.sharedSecret).
WithAdGroups(rb.adGroups).
WithAdUsers(rb.adUsers).
WithReaderAdGroups(rb.readerAdGroups).
WithReaderAdUsers(rb.readerAdUsers).
WithOwner(rb.owner).
WithCreator(rb.creator).
WithWBS(rb.wbs).
Expand Down
10 changes: 10 additions & 0 deletions api/applications/models/application_registration_patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,21 @@ type ApplicationRegistrationPatch struct {
// required: false
AdGroups *[]string `json:"adGroups,omitempty"`

// AdUsers the users/service-principals that should be able to access the application
//
// required: false
AdUsers *[]string `json:"adUsers,omitempty"`

// ReaderAdGroups the groups that should be able to read the application
//
// required: false
ReaderAdGroups *[]string `json:"readerAdGroups,omitempty"`

// ReaderAdUsers the users/service-principals that should be able to read the application
//
// required: false
ReaderAdUsers *[]string `json:"readerAdUsers,omitempty"`

// Owner of the application - should be an email
//
// required: false
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ require (
github.com/cert-manager/cert-manager v1.15.0
github.com/equinor/radix-common v1.9.5
github.com/equinor/radix-job-scheduler v1.11.0
github.com/equinor/radix-operator v1.59.1
github.com/equinor/radix-operator v1.61.0
github.com/evanphx/json-patch/v5 v5.9.0
github.com/felixge/httpsnoop v1.0.4
github.com/golang-jwt/jwt/v5 v5.2.1
github.com/golang/mock v1.6.0
github.com/google/uuid v1.6.0
github.com/gorilla/mux v1.8.1
github.com/kedacore/keda/v2 v2.15.1
github.com/marstr/guid v1.1.0
Expand Down Expand Up @@ -64,7 +65,6 @@ require (
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/go-containerregistry v0.16.1 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-multierror v1.1.1 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ github.com/equinor/radix-common v1.9.5 h1:p1xldkYUoavwIMguoxxOyVkOXLPA6K8qMsgzez
github.com/equinor/radix-common v1.9.5/go.mod h1:+g0Wj0D40zz29DjNkYKVmCVeYy4OsFWKI7Qi9rA6kpY=
github.com/equinor/radix-job-scheduler v1.11.0 h1:8wCmXOVl/1cto8q2WJQEE06Cw68/QmfoifYVR49vzkY=
github.com/equinor/radix-job-scheduler v1.11.0/go.mod h1:yPXn3kDcMY0Z3kBkosjuefsdY1x2g0NlBeybMmHz5hc=
github.com/equinor/radix-operator v1.59.1 h1:wzb2tF4MWtGDWmyYuIv3oh17G5Bx8ztW9O+WgnI3QFc=
github.com/equinor/radix-operator v1.59.1/go.mod h1:uRW9SgVZ94hkpq87npVv2YVviRuXNJ1zgCleya1uvr8=
github.com/equinor/radix-operator v1.61.0 h1:kHWHn5p9S+wKqOTSKtju2URW5FKgAFng1p6RLRnaTmE=
github.com/equinor/radix-operator v1.61.0/go.mod h1:uRW9SgVZ94hkpq87npVv2YVviRuXNJ1zgCleya1uvr8=
github.com/evanphx/json-patch v5.9.0+incompatible h1:fBXyNpNMuTTDdquAq/uisOr2lShz4oaXpDTX2bLe7ls=
github.com/evanphx/json-patch v5.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg=
Expand Down
35 changes: 35 additions & 0 deletions swaggerui/html/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -5307,6 +5307,9 @@
"repository",
"sharedSecret",
"adGroups",
"adUsers",
"readerAdGroups",
"readerAdUsers",
"owner",
"creator",
"configBranch"
Expand All @@ -5320,6 +5323,14 @@
},
"x-go-name": "AdGroups"
},
"adUsers": {
"description": "AdUsers the users/service-principals that should be able to access the application",
"type": "array",
"items": {
"type": "string"
},
"x-go-name": "AdUsers"
},
"configBranch": {
"description": "ConfigBranch information",
"type": "string",
Expand Down Expand Up @@ -5359,6 +5370,14 @@
},
"x-go-name": "ReaderAdGroups"
},
"readerAdUsers": {
"description": "ReaderAdUsers the users/service-principals that should be able to read the application",
"type": "array",
"items": {
"type": "string"
},
"x-go-name": "ReaderAdUsers"
},
"repository": {
"description": "Repository the github repository",
"type": "string",
Expand Down Expand Up @@ -5390,6 +5409,14 @@
},
"x-go-name": "AdGroups"
},
"adUsers": {
"description": "AdUsers the users/service-principals that should be able to access the application",
"type": "array",
"items": {
"type": "string"
},
"x-go-name": "AdUsers"
},
"configBranch": {
"description": "ConfigBranch information",
"type": "string",
Expand Down Expand Up @@ -5418,6 +5445,14 @@
},
"x-go-name": "ReaderAdGroups"
},
"readerAdUsers": {
"description": "ReaderAdUsers the users/service-principals that should be able to read the application",
"type": "array",
"items": {
"type": "string"
},
"x-go-name": "ReaderAdUsers"
},
"repository": {
"description": "Repository the github repository",
"type": "string",
Expand Down

0 comments on commit 9cc57b2

Please sign in to comment.