Skip to content

Commit ab405e7

Browse files
d0weinbergerd0weinberger
committed
refactor(settings): settings client uses permission handling from core lib
When creating a new settings client, the nested rest client is also used to create the core lib permission client.
1 parent 4b8b6b8 commit ab405e7

File tree

2 files changed

+56
-88
lines changed

2 files changed

+56
-88
lines changed

‎pkg/client/dtclient/settings_client.go

Lines changed: 26 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232

3333
coreapi "github.com/dynatrace/dynatrace-configuration-as-code-core/api"
3434
corerest "github.com/dynatrace/dynatrace-configuration-as-code-core/api/rest"
35+
coresettings "github.com/dynatrace/dynatrace-configuration-as-code-core/clients/settings/permissions"
3536
"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/cache"
3637
"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags"
3738
"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/filter"
@@ -174,6 +175,9 @@ type SettingsClient struct {
174175
// client is a rest client used to target platform enabled environments
175176
client *corerest.Client
176177

178+
// permissionClient is used for CRUD operations on settings object permissions
179+
permissionClient *coresettings.Client
180+
177181
// settingsSchemaAPIPath is the API path to use for accessing settings schemas
178182
settingsSchemaAPIPath string
179183

@@ -247,10 +251,18 @@ type (
247251

248252
Accessor struct {
249253
Type TypeAccessor `json:"type"`
254+
// ID is only needed and valid for TypeAccessor Group and User, but not for AllUsers
255+
ID string `json:"id,omitempty"`
250256
}
257+
// PermissionObject contains the Accessor, and a slice of Permissions.
251258
PermissionObject struct {
259+
Accessor *Accessor `json:"accessor,omitempty"`
260+
261+
// Explanation:
262+
// - If Permissions is empty, the Accessor has no permissions
263+
// - If Permissions contains "r", the Accessor has read access
264+
// - If Permissions contains "w", the Accessor has write access
252265
Permissions []TypePermissions `json:"permissions"`
253-
Accessor *Accessor `json:"accessor,omitempty"`
254266
}
255267

256268
postResponse struct {
@@ -282,12 +294,10 @@ type (
282294
)
283295

284296
const (
285-
settingsSchemaAPIPathClassic = "/api/v2/settings/schemas"
286-
settingsSchemaAPIPathPlatform = "/platform/classic/environment-api/v2/settings/schemas"
287-
settingsObjectAPIPathClassic = "/api/v2/settings/objects"
288-
settingsObjectAPIPathPlatform = "/platform/classic/environment-api/v2/settings/objects"
289-
settingsPermissionAllUsersAPIPath = "/platform/classic/environment-api/v2/settings/objects/{objectId}/permissions/all-users" // GET, PUT, DELETE
290-
settingsPermissionAPIPath = "/platform/classic/environment-api/v2/settings/objects/{objectId}/permissions" // POST
297+
settingsSchemaAPIPathClassic = "/api/v2/settings/schemas"
298+
settingsSchemaAPIPathPlatform = "/platform/classic/environment-api/v2/settings/schemas"
299+
settingsObjectAPIPathClassic = "/api/v2/settings/objects"
300+
settingsObjectAPIPathPlatform = "/platform/classic/environment-api/v2/settings/objects"
291301
)
292302

293303
func WithExternalIDGenerator(g idutils.ExternalIDGenerator) func(client *SettingsClient) {
@@ -350,6 +360,7 @@ func NewPlatformSettingsClient(client *corerest.Client, opts ...func(dynatraceCl
350360
d := &SettingsClient{
351361
serverVersion: version.Version{},
352362
client: client,
363+
permissionClient: coresettings.NewClient(client),
353364
retrySettings: DefaultRetrySettings,
354365
settingsSchemaAPIPath: settingsSchemaAPIPathPlatform,
355366
settingsObjectAPIPath: settingsObjectAPIPathPlatform,
@@ -373,6 +384,7 @@ func NewClassicSettingsClient(client *corerest.Client, opts ...func(dynatraceCli
373384
d := &SettingsClient{
374385
serverVersion: version.Version{},
375386
client: client,
387+
permissionClient: coresettings.NewClient(client),
376388
retrySettings: DefaultRetrySettings,
377389
settingsSchemaAPIPath: settingsSchemaAPIPathClassic,
378390
settingsObjectAPIPath: settingsObjectAPIPathClassic,
@@ -626,7 +638,7 @@ func (d *SettingsClient) modifyPermission(ctx context.Context, objectID string,
626638
if permissions != nil {
627639
return d.UpsertPermission(ctx, objectID, PermissionObject{
628640
Permissions: permissions,
629-
Accessor: &Accessor{AllUsers},
641+
Accessor: &Accessor{Type: AllUsers},
630642
})
631643
}
632644

@@ -937,16 +949,7 @@ func (d *SettingsClient) Delete(ctx context.Context, objectID string) error {
937949
}
938950

939951
func (d *SettingsClient) GetPermission(ctx context.Context, objectID string) (PermissionObject, error) {
940-
path, err := getPermissionPathWithID(objectID)
941-
if err != nil {
942-
return PermissionObject{}, err
943-
}
944-
945-
resp, err := coreapi.AsResponseOrError(d.client.GET(
946-
ctx,
947-
path,
948-
corerest.RequestOptions{CustomShouldRetryFunc: corerest.RetryIfTooManyRequests},
949-
))
952+
resp, err := d.permissionClient.GetAllUsersAccessor(ctx, objectID)
950953

951954
// when the API returns a 404 it means that you don't have permission (no-access), or the object does not exist
952955
if coreapi.IsNotFoundError(err) {
@@ -974,47 +977,24 @@ func (d *SettingsClient) UpsertPermission(ctx context.Context, objectID string,
974977
return fmt.Errorf("failed to marshal permission: %w", err)
975978
}
976979

977-
path, err := getPermissionPathWithID(objectID)
978-
if err != nil {
979-
return err
980-
}
981-
982-
_, err = coreapi.AsResponseOrError(d.client.GET(
983-
ctx,
984-
path,
985-
corerest.RequestOptions{CustomShouldRetryFunc: corerest.RetryIfTooManyRequests},
986-
))
980+
_, err = d.permissionClient.GetAllUsersAccessor(ctx, objectID)
987981

988982
// When there is no error the object is found and we update it.
989983
if err == nil {
990-
_, err = coreapi.AsResponseOrError(d.client.PUT(
991-
ctx,
992-
path,
993-
bytes.NewReader(payload),
994-
corerest.RequestOptions{CustomShouldRetryFunc: corerest.RetryIfTooManyRequests},
995-
))
996-
984+
_, err = d.permissionClient.UpdateAllUsersAccessor(ctx, objectID, payload)
997985
if err != nil {
998986
return fmt.Errorf("failed to update permission: %w", err)
999987
}
1000988

1001989
return nil
1002990
}
1003991

1004-
// On a 404 we step throw and try to create the object on remote in any other error we exit
992+
// On a 404 we step through and try to create the object on remote in any other error we exit
1005993
if !coreapi.IsNotFoundError(err) {
1006994
return fmt.Errorf("failed to get permission: %w", err)
1007995
}
1008996

1009-
// endpoint for POST is different to PUT
1010-
path = strings.ReplaceAll(settingsPermissionAPIPath, "{objectId}", objectID)
1011-
_, err = coreapi.AsResponseOrError(d.client.POST(
1012-
ctx,
1013-
path,
1014-
bytes.NewReader(payload),
1015-
corerest.RequestOptions{CustomShouldRetryFunc: corerest.RetryIfTooManyRequests},
1016-
))
1017-
997+
_, err = d.permissionClient.Create(ctx, objectID, payload)
1018998
if err != nil {
1019999
return fmt.Errorf("failed to create permission: %w", err)
10201000
}
@@ -1023,28 +1003,11 @@ func (d *SettingsClient) UpsertPermission(ctx context.Context, objectID string,
10231003
}
10241004

10251005
func (d *SettingsClient) DeletePermission(ctx context.Context, objectID string) error {
1026-
path, err := getPermissionPathWithID(objectID)
1027-
if err != nil {
1028-
return err
1029-
}
1030-
_, err = coreapi.AsResponseOrError(d.client.DELETE(
1031-
ctx,
1032-
path,
1033-
corerest.RequestOptions{CustomShouldRetryFunc: corerest.RetryIfTooManyRequests},
1034-
))
1006+
_, err := d.permissionClient.DeleteAllUsersAccessor(ctx, objectID)
10351007

10361008
// deployments with "none" for all-user will always try to delete. This could be an update (restricted to shared) or it stays the same (delete 404)
10371009
if err != nil && !coreapi.IsNotFoundError(err) {
10381010
return fmt.Errorf("failed to delete permission object: %w", err)
10391011
}
10401012
return nil
10411013
}
1042-
1043-
// When using the settingsPermissionAllUsersAPIPath we need to add the objectID into path
1044-
func getPermissionPathWithID(id string) (string, error) {
1045-
if id == "" {
1046-
return "", fmt.Errorf("id cannot be empty")
1047-
}
1048-
1049-
return strings.ReplaceAll(settingsPermissionAllUsersAPIPath, "{objectId}", id), nil
1050-
}

‎pkg/client/dtclient/settings_client_test.go

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -961,11 +961,11 @@ func TestUpsertSettings_ACL(t *testing.T) {
961961
require.NoError(t, err)
962962
})
963963

964-
mux.HandleFunc(settingsPermissionAPIPath, func(w http.ResponseWriter, r *http.Request) {
964+
mux.HandleFunc("/platform/classic/environment-api/v2/settings/objects/{objectId}/permissions", func(w http.ResponseWriter, r *http.Request) {
965965
t.Errorf("Called '%s' but it should not be called", r.Pattern)
966966
})
967967

968-
mux.HandleFunc(settingsPermissionAllUsersAPIPath, func(w http.ResponseWriter, r *http.Request) {
968+
mux.HandleFunc("/platform/classic/environment-api/v2/settings/objects/{objectId}/permissions/all-users", func(w http.ResponseWriter, r *http.Request) {
969969
t.Errorf("Called '%s' but it should not be called", r.Pattern)
970970
})
971971

@@ -1014,11 +1014,11 @@ func TestUpsertSettings_ACL(t *testing.T) {
10141014
require.NoError(t, err)
10151015
})
10161016

1017-
mux.HandleFunc(settingsPermissionAPIPath, func(w http.ResponseWriter, r *http.Request) {
1017+
mux.HandleFunc("/platform/classic/environment-api/v2/settings/objects/{objectId}/permissions", func(w http.ResponseWriter, r *http.Request) {
10181018
t.Errorf("Called '%s' but it should not be called", r.Pattern)
10191019
})
10201020

1021-
mux.HandleFunc(settingsPermissionAllUsersAPIPath, func(w http.ResponseWriter, r *http.Request) {
1021+
mux.HandleFunc("/platform/classic/environment-api/v2/settings/objects/{objectId}/permissions/all-users", func(w http.ResponseWriter, r *http.Request) {
10221022
t.Errorf("Called '%s' but it should not be called", r.Pattern)
10231023
})
10241024

@@ -1068,11 +1068,11 @@ func TestUpsertSettings_ACL(t *testing.T) {
10681068
require.NoError(t, err)
10691069
})
10701070

1071-
mux.HandleFunc(settingsPermissionAPIPath, func(w http.ResponseWriter, r *http.Request) {
1071+
mux.HandleFunc("/platform/classic/environment-api/v2/settings/objects/{objectId}/permissions", func(w http.ResponseWriter, r *http.Request) {
10721072
t.Errorf("Called '%s' but it should not be called", r.Pattern)
10731073
})
10741074

1075-
mux.HandleFunc("DELETE "+settingsPermissionAllUsersAPIPath, func(w http.ResponseWriter, r *http.Request) {
1075+
mux.HandleFunc("DELETE "+"/platform/classic/environment-api/v2/settings/objects/{objectId}/permissions/all-users", func(w http.ResponseWriter, r *http.Request) {
10761076
deleteCalled = true
10771077
_, err := w.Write([]byte("{}"))
10781078
require.NoError(t, err)
@@ -1121,19 +1121,19 @@ func TestUpsertSettings_ACL(t *testing.T) {
11211121
require.NoError(t, err)
11221122
})
11231123

1124-
mux.HandleFunc("POST "+settingsPermissionAPIPath, func(w http.ResponseWriter, r *http.Request) {
1124+
mux.HandleFunc("POST /platform/classic/environment-api/v2/settings/objects/{objectId}/permissions", func(w http.ResponseWriter, r *http.Request) {
11251125
postPermissionCalled = true
11261126
_, err := w.Write([]byte("{}"))
11271127
require.NoError(t, err)
11281128
})
11291129

1130-
mux.HandleFunc("GET "+settingsPermissionAllUsersAPIPath, func(w http.ResponseWriter, r *http.Request) {
1130+
mux.HandleFunc("GET /platform/classic/environment-api/v2/settings/objects/{objectId}/permissions/all-users", func(w http.ResponseWriter, r *http.Request) {
11311131
w.WriteHeader(http.StatusNotFound)
11321132
_, err := w.Write([]byte("{}"))
11331133
require.NoError(t, err)
11341134
})
11351135

1136-
mux.HandleFunc(settingsPermissionAllUsersAPIPath, func(w http.ResponseWriter, r *http.Request) {
1136+
mux.HandleFunc("/platform/classic/environment-api/v2/settings/objects/{objectId}/permissions/all-users", func(w http.ResponseWriter, r *http.Request) {
11371137
t.Errorf("Called '%s' but it should not be called", r.Pattern)
11381138
})
11391139

@@ -1180,16 +1180,16 @@ func TestUpsertSettings_ACL(t *testing.T) {
11801180
require.NoError(t, err)
11811181
})
11821182

1183-
mux.HandleFunc(settingsPermissionAPIPath, func(w http.ResponseWriter, r *http.Request) {
1183+
mux.HandleFunc("/platform/classic/environment-api/v2/settings/objects/{objectId}/permissions", func(w http.ResponseWriter, r *http.Request) {
11841184
t.Errorf("Called '%s' but it should not be called", r.Pattern)
11851185
})
11861186

1187-
mux.HandleFunc("GET "+settingsPermissionAllUsersAPIPath, func(w http.ResponseWriter, r *http.Request) {
1187+
mux.HandleFunc("GET /platform/classic/environment-api/v2/settings/objects/{objectId}/permissions/all-users", func(w http.ResponseWriter, r *http.Request) {
11881188
_, err := w.Write([]byte("{}"))
11891189
require.NoError(t, err)
11901190
})
11911191

1192-
mux.HandleFunc("PUT "+settingsPermissionAllUsersAPIPath, func(w http.ResponseWriter, r *http.Request) {
1192+
mux.HandleFunc("PUT /platform/classic/environment-api/v2/settings/objects/{objectId}/permissions/all-users", func(w http.ResponseWriter, r *http.Request) {
11931193
putPermissionCalled = true
11941194
_, err := w.Write([]byte("{}"))
11951195
require.NoError(t, err)
@@ -2542,7 +2542,7 @@ func TestSettingsClient_GetPermission(t *testing.T) {
25422542
},
25432543
expectedResponse: PermissionObject{
25442544
Permissions: []TypePermissions{},
2545-
Accessor: &Accessor{Type: TypeAccessor(AllUsers)},
2545+
Accessor: &Accessor{Type: AllUsers},
25462546
},
25472547
},
25482548
{
@@ -2555,7 +2555,7 @@ func TestSettingsClient_GetPermission(t *testing.T) {
25552555
},
25562556
expectedResponse: PermissionObject{
25572557
Permissions: []TypePermissions{Read},
2558-
Accessor: &Accessor{Type: TypeAccessor(AllUsers)},
2558+
Accessor: &Accessor{Type: AllUsers},
25592559
},
25602560
},
25612561
{
@@ -2568,7 +2568,7 @@ func TestSettingsClient_GetPermission(t *testing.T) {
25682568
},
25692569
expectedResponse: PermissionObject{
25702570
Permissions: []TypePermissions{Read, Write},
2571-
Accessor: &Accessor{Type: TypeAccessor(AllUsers)},
2571+
Accessor: &Accessor{Type: AllUsers},
25722572
},
25732573
},
25742574
}
@@ -2656,7 +2656,7 @@ func TestSettingsClient_UpsertPermission(t *testing.T) {
26562656
tests := []struct {
26572657
name string
26582658
id string
2659-
permissionObject PermissionObject
2659+
PermissionObject PermissionObject
26602660
responses []testutils.ResponseDef
26612661
}{
26622662
{
@@ -2682,8 +2682,9 @@ func TestSettingsClient_UpsertPermission(t *testing.T) {
26822682
},
26832683
},
26842684
},
2685-
permissionObject: PermissionObject{
2685+
PermissionObject: PermissionObject{
26862686
Permissions: []TypePermissions{Read, Write},
2687+
Accessor: &Accessor{Type: AllUsers},
26872688
},
26882689
},
26892690
{
@@ -2706,8 +2707,9 @@ func TestSettingsClient_UpsertPermission(t *testing.T) {
27062707
},
27072708
},
27082709
},
2709-
permissionObject: PermissionObject{
2710+
PermissionObject: PermissionObject{
27102711
Permissions: []TypePermissions{Write},
2712+
Accessor: &Accessor{Type: AllUsers},
27112713
},
27122714
},
27132715
}
@@ -2719,7 +2721,7 @@ func TestSettingsClient_UpsertPermission(t *testing.T) {
27192721
dcl, err := NewPlatformSettingsClient(corerest.NewClient(server.URL(), server.Client()))
27202722
assert.NoError(t, err)
27212723

2722-
err = dcl.UpsertPermission(t.Context(), tt.id, tt.permissionObject)
2724+
err = dcl.UpsertPermission(t.Context(), tt.id, tt.PermissionObject)
27232725
assert.NoError(t, err)
27242726
})
27252727
}
@@ -2728,7 +2730,7 @@ func TestSettingsClient_UpsertPermission(t *testing.T) {
27282730
tests := []struct {
27292731
name string
27302732
id string
2731-
permissionObject PermissionObject
2733+
PermissionObject PermissionObject
27322734
responses []testutils.ResponseDef
27332735
}{
27342736
{
@@ -2751,8 +2753,9 @@ func TestSettingsClient_UpsertPermission(t *testing.T) {
27512753
},
27522754
},
27532755
},
2754-
permissionObject: PermissionObject{
2756+
PermissionObject: PermissionObject{
27552757
Permissions: []TypePermissions{Read, Write},
2758+
Accessor: &Accessor{Type: AllUsers},
27562759
},
27572760
},
27582761
{
@@ -2775,8 +2778,9 @@ func TestSettingsClient_UpsertPermission(t *testing.T) {
27752778
},
27762779
},
27772780
},
2778-
permissionObject: PermissionObject{
2781+
PermissionObject: PermissionObject{
27792782
Permissions: []TypePermissions{Read, Write},
2783+
Accessor: &Accessor{Type: AllUsers},
27802784
},
27812785
},
27822786
{
@@ -2792,8 +2796,9 @@ func TestSettingsClient_UpsertPermission(t *testing.T) {
27922796
},
27932797
},
27942798
},
2795-
permissionObject: PermissionObject{
2799+
PermissionObject: PermissionObject{
27962800
Permissions: []TypePermissions{Read, Write},
2801+
Accessor: &Accessor{Type: AllUsers},
27972802
},
27982803
},
27992804
{
@@ -2809,7 +2814,7 @@ func TestSettingsClient_UpsertPermission(t *testing.T) {
28092814
},
28102815
},
28112816
},
2812-
permissionObject: PermissionObject{},
2817+
PermissionObject: PermissionObject{},
28132818
},
28142819
}
28152820
for _, tt := range tests {
@@ -2820,7 +2825,7 @@ func TestSettingsClient_UpsertPermission(t *testing.T) {
28202825
dcl, err := NewPlatformSettingsClient(corerest.NewClient(server.URL(), server.Client()))
28212826
assert.NoError(t, err)
28222827

2823-
err = dcl.UpsertPermission(t.Context(), tt.id, tt.permissionObject)
2828+
err = dcl.UpsertPermission(t.Context(), tt.id, tt.PermissionObject)
28242829
assert.Error(t, err)
28252830
})
28262831
}

0 commit comments

Comments
 (0)