Compare commits

...

4 Commits

Author SHA1 Message Date
Andrew Nester ad2fc1320f
Merge 20f33d6397 into 0753dfe2f4 2024-10-15 13:15:34 +00:00
Andrew Nester 0753dfe2f4
Fixed unmarshalling json input into `interface{}` type (#1832)
## Changes
Fixed unmarshalling json input into `interface{}` type

Commands like `api post` support free form request input, so it should
be unmarshaled correctly

## Tests
Added regression test + E2E test pass
2024-10-15 12:10:02 +00:00
Andrew Nester 20f33d6397
fixes 2024-10-10 15:24:52 +02:00
Andrew Nester 8f145e0294
Use SetPermissions instead of UpdatePermissions when setting folder permissions based on top-level ones 2024-10-10 14:13:44 +02:00
4 changed files with 190 additions and 18 deletions

View File

@ -3,6 +3,7 @@ package permissions
import ( import (
"context" "context"
"fmt" "fmt"
"strings"
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/diag"
@ -52,16 +53,60 @@ func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) error {
} }
w := b.WorkspaceClient().Workspace w := b.WorkspaceClient().Workspace
obj, err := w.GetStatusByPath(ctx, b.Config.Workspace.RootPath) err := setPermissions(ctx, w, b.Config.Workspace.RootPath, permissions)
if err != nil { if err != nil {
return err return err
} }
_, err = w.UpdatePermissions(ctx, workspace.WorkspaceObjectPermissionsRequest{ // Adding backslash to the root path
rootPath := b.Config.Workspace.RootPath
if rootPath[len(rootPath)-1] != '/' {
rootPath += "/"
}
if !strings.HasPrefix(b.Config.Workspace.ArtifactPath, rootPath) {
err = setPermissions(ctx, w, b.Config.Workspace.ArtifactPath, permissions)
if err != nil {
return err
}
}
if !strings.HasPrefix(b.Config.Workspace.FilePath, rootPath) {
err = setPermissions(ctx, w, b.Config.Workspace.FilePath, permissions)
if err != nil {
return err
}
}
if !strings.HasPrefix(b.Config.Workspace.StatePath, rootPath) {
err = setPermissions(ctx, w, b.Config.Workspace.StatePath, permissions)
if err != nil {
return err
}
}
if !strings.HasPrefix(b.Config.Workspace.ResourcePath, rootPath) {
err = setPermissions(ctx, w, b.Config.Workspace.ResourcePath, permissions)
if err != nil {
return err
}
}
return err
}
func setPermissions(ctx context.Context, w workspace.WorkspaceInterface, path string, permissions []workspace.WorkspaceObjectAccessControlRequest) error {
obj, err := w.GetStatusByPath(ctx, path)
if err != nil {
return err
}
_, err = w.SetPermissions(ctx, workspace.WorkspaceObjectPermissionsRequest{
WorkspaceObjectId: fmt.Sprint(obj.ObjectId), WorkspaceObjectId: fmt.Sprint(obj.ObjectId),
WorkspaceObjectType: "directories", WorkspaceObjectType: "directories",
AccessControlList: permissions, AccessControlList: permissions,
}) })
return err return err
} }

View File

@ -21,7 +21,11 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) {
b := &bundle.Bundle{ b := &bundle.Bundle{
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
RootPath: "/Users/foo@bar.com", RootPath: "/Users/foo@bar.com",
ArtifactPath: "/Users/foo@bar.com/artifacts",
FilePath: "/Users/foo@bar.com/files",
StatePath: "/Users/foo@bar.com/state",
ResourcePath: "/Users/foo@bar.com/resources",
}, },
Permissions: []resources.Permission{ Permissions: []resources.Permission{
{Level: CAN_MANAGE, UserName: "TestUser"}, {Level: CAN_MANAGE, UserName: "TestUser"},
@ -59,7 +63,7 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) {
workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/foo@bar.com").Return(&workspace.ObjectInfo{ workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/foo@bar.com").Return(&workspace.ObjectInfo{
ObjectId: 1234, ObjectId: 1234,
}, nil) }, nil)
workspaceApi.EXPECT().UpdatePermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{ workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{ AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
{UserName: "TestUser", PermissionLevel: "CAN_MANAGE"}, {UserName: "TestUser", PermissionLevel: "CAN_MANAGE"},
{GroupName: "TestGroup", PermissionLevel: "CAN_READ"}, {GroupName: "TestGroup", PermissionLevel: "CAN_READ"},
@ -72,3 +76,116 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) {
diags := bundle.Apply(context.Background(), b, ApplyWorkspaceRootPermissions()) diags := bundle.Apply(context.Background(), b, ApplyWorkspaceRootPermissions())
require.NoError(t, diags.Error()) require.NoError(t, diags.Error())
} }
func TestApplyWorkspaceRootPermissionsForAllPaths(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Workspace: config.Workspace{
RootPath: "/Some/Root/Path",
ArtifactPath: "/Users/foo@bar.com/artifacts",
FilePath: "/Users/foo@bar.com/files",
StatePath: "/Users/foo@bar.com/state",
ResourcePath: "/Users/foo@bar.com/resources",
},
Permissions: []resources.Permission{
{Level: CAN_MANAGE, UserName: "TestUser"},
{Level: CAN_VIEW, GroupName: "TestGroup"},
{Level: CAN_RUN, ServicePrincipalName: "TestServicePrincipal"},
},
Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job_1": {JobSettings: &jobs.JobSettings{Name: "job_1"}},
"job_2": {JobSettings: &jobs.JobSettings{Name: "job_2"}},
},
Pipelines: map[string]*resources.Pipeline{
"pipeline_1": {PipelineSpec: &pipelines.PipelineSpec{}},
"pipeline_2": {PipelineSpec: &pipelines.PipelineSpec{}},
},
Models: map[string]*resources.MlflowModel{
"model_1": {Model: &ml.Model{}},
"model_2": {Model: &ml.Model{}},
},
Experiments: map[string]*resources.MlflowExperiment{
"experiment_1": {Experiment: &ml.Experiment{}},
"experiment_2": {Experiment: &ml.Experiment{}},
},
ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{
"endpoint_1": {CreateServingEndpoint: &serving.CreateServingEndpoint{}},
"endpoint_2": {CreateServingEndpoint: &serving.CreateServingEndpoint{}},
},
},
},
}
m := mocks.NewMockWorkspaceClient(t)
b.SetWorkpaceClient(m.WorkspaceClient)
workspaceApi := m.GetMockWorkspaceAPI()
workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Some/Root/Path").Return(&workspace.ObjectInfo{
ObjectId: 1,
}, nil)
workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/foo@bar.com/artifacts").Return(&workspace.ObjectInfo{
ObjectId: 2,
}, nil)
workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/foo@bar.com/files").Return(&workspace.ObjectInfo{
ObjectId: 3,
}, nil)
workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/foo@bar.com/state").Return(&workspace.ObjectInfo{
ObjectId: 4,
}, nil)
workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/foo@bar.com/resources").Return(&workspace.ObjectInfo{
ObjectId: 5,
}, nil)
workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
{UserName: "TestUser", PermissionLevel: "CAN_MANAGE"},
{GroupName: "TestGroup", PermissionLevel: "CAN_READ"},
{ServicePrincipalName: "TestServicePrincipal", PermissionLevel: "CAN_RUN"},
},
WorkspaceObjectId: "1",
WorkspaceObjectType: "directories",
}).Return(nil, nil)
workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
{UserName: "TestUser", PermissionLevel: "CAN_MANAGE"},
{GroupName: "TestGroup", PermissionLevel: "CAN_READ"},
{ServicePrincipalName: "TestServicePrincipal", PermissionLevel: "CAN_RUN"},
},
WorkspaceObjectId: "2",
WorkspaceObjectType: "directories",
}).Return(nil, nil)
workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
{UserName: "TestUser", PermissionLevel: "CAN_MANAGE"},
{GroupName: "TestGroup", PermissionLevel: "CAN_READ"},
{ServicePrincipalName: "TestServicePrincipal", PermissionLevel: "CAN_RUN"},
},
WorkspaceObjectId: "3",
WorkspaceObjectType: "directories",
}).Return(nil, nil)
workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
{UserName: "TestUser", PermissionLevel: "CAN_MANAGE"},
{GroupName: "TestGroup", PermissionLevel: "CAN_READ"},
{ServicePrincipalName: "TestServicePrincipal", PermissionLevel: "CAN_RUN"},
},
WorkspaceObjectId: "4",
WorkspaceObjectType: "directories",
}).Return(nil, nil)
workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
{UserName: "TestUser", PermissionLevel: "CAN_MANAGE"},
{GroupName: "TestGroup", PermissionLevel: "CAN_READ"},
{ServicePrincipalName: "TestServicePrincipal", PermissionLevel: "CAN_RUN"},
},
WorkspaceObjectId: "5",
WorkspaceObjectType: "directories",
}).Return(nil, nil)
diags := bundle.Apply(context.Background(), b, ApplyWorkspaceRootPermissions())
require.NoError(t, diags.Error())
}

View File

@ -4,6 +4,7 @@ import (
"encoding/json" "encoding/json"
"fmt" "fmt"
"os" "os"
"reflect"
"github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn/convert" "github.com/databricks/cli/libs/dyn/convert"
@ -63,11 +64,24 @@ func (j *JsonFlag) Unmarshal(v any) diag.Diagnostics {
return diags.Extend(diag.FromErr(err)) return diags.Extend(diag.FromErr(err))
} }
// Finally unmarshal the normalized data to the output. kind := reflect.ValueOf(v).Kind()
// It will fill in the ForceSendFields field if the struct contains it. if kind == reflect.Ptr {
err = marshal.Unmarshal(data, v) kind = reflect.ValueOf(v).Elem().Kind()
if err != nil { }
return diags.Extend(diag.FromErr(err))
if kind == reflect.Struct {
// Finally unmarshal the normalized data to the output.
// It will fill in the ForceSendFields field if the struct contains it.
err = marshal.Unmarshal(data, v)
if err != nil {
return diags.Extend(diag.FromErr(err))
}
} else {
// If the output is not a struct, just unmarshal the data to the output.
err = json.Unmarshal(data, v)
if err != nil {
return diags.Extend(diag.FromErr(err))
}
} }
return diags return diags

View File

@ -13,10 +13,6 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
type requestType struct {
Foo string `json:"foo"`
}
func TestJsonFlagEmpty(t *testing.T) { func TestJsonFlagEmpty(t *testing.T) {
var body JsonFlag var body JsonFlag
@ -35,13 +31,13 @@ func TestJsonFlagInline(t *testing.T) {
err := body.Set(`{"foo": "bar"}`) err := body.Set(`{"foo": "bar"}`)
assert.NoError(t, err) assert.NoError(t, err)
var request requestType var request any
diags := body.Unmarshal(&request) diags := body.Unmarshal(&request)
assert.NoError(t, diags.Error()) assert.NoError(t, diags.Error())
assert.Empty(t, diags) assert.Empty(t, diags)
assert.Equal(t, "JSON (14 bytes)", body.String()) assert.Equal(t, "JSON (14 bytes)", body.String())
assert.Equal(t, requestType{"bar"}, request) assert.Equal(t, map[string]any{"foo": "bar"}, request)
} }
func TestJsonFlagError(t *testing.T) { func TestJsonFlagError(t *testing.T) {
@ -50,7 +46,7 @@ func TestJsonFlagError(t *testing.T) {
err := body.Set(`{"foo":`) err := body.Set(`{"foo":`)
assert.NoError(t, err) assert.NoError(t, err)
var request requestType var request any
diags := body.Unmarshal(&request) diags := body.Unmarshal(&request)
assert.EqualError(t, diags.Error(), "error decoding JSON at (inline):1:8: unexpected end of JSON input") assert.EqualError(t, diags.Error(), "error decoding JSON at (inline):1:8: unexpected end of JSON input")
assert.Equal(t, "JSON (7 bytes)", body.String()) assert.Equal(t, "JSON (7 bytes)", body.String())
@ -58,7 +54,7 @@ func TestJsonFlagError(t *testing.T) {
func TestJsonFlagFile(t *testing.T) { func TestJsonFlagFile(t *testing.T) {
var body JsonFlag var body JsonFlag
var request requestType var request any
var fpath string var fpath string
var payload = []byte(`{"foo": "bar"}`) var payload = []byte(`{"foo": "bar"}`)
@ -78,7 +74,7 @@ func TestJsonFlagFile(t *testing.T) {
assert.NoError(t, diags.Error()) assert.NoError(t, diags.Error())
assert.Empty(t, diags) assert.Empty(t, diags)
assert.Equal(t, requestType{"bar"}, request) assert.Equal(t, map[string]any{"foo": "bar"}, request)
} }
const jsonData = ` const jsonData = `