fixes

bundle/permissions/workspace_root.go

@@ -13,22 +13,27 @@ import (
 type workspaceRootPermissions struct {
 }
 
+type validateSharedRootPermissions struct {
+}
+
 func ApplyWorkspaceRootPermissions() bundle.Mutator {
 	return &workspaceRootPermissions{}
 }
 
+func (*workspaceRootPermissions) Name() string {
+	return "ApplyWorkspaceRootPermissions"
+}
+
+func ValidateSharedRootPermissions() bundle.Mutator {
+	return &validateSharedRootPermissions{}
+}
+
+func (*validateSharedRootPermissions) Name() string {
+	return "ValidateSharedRootPermissions"
+}
+
 // Apply implements bundle.Mutator.
 func (*workspaceRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
-
-	if isWorkspaceSharedRoot(b.Config.Workspace.RootPath) {
-		diags := checkWorkspaceRootPermissions(b)
-		// If there are permissions warnings, return them and do not apply permissions
-		// because they are not set correctly for /Workspace/Shared root anyway
-		if len(diags) > 0 {
-			return diags
-		}
-	}
-
 	err := giveAccessForWorkspaceRoot(ctx, b)
 	if err != nil {
 		return diag.FromErr(err)
@@ -37,8 +42,12 @@ func (*workspaceRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) di
 	return nil
 }
 
-func (*workspaceRootPermissions) Name() string {
-	return "ApplyWorkspaceRootPermissions"
+func (*validateSharedRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
+	if isWorkspaceSharedRoot(b.Config.Workspace.RootPath) {
+		return isUsersGroupPermissionSet(b)
+	}
+
+	return nil
 }
 
 func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) error {
@@ -93,14 +102,15 @@ func isWorkspaceSharedRoot(path string) bool {
 	return strings.HasPrefix(path, "/Workspace/Shared/")
 }
 
-// checkWorkspaceRootPermissions checks that if permissions are set for the workspace root, and workspace root starts with /Workspace/Shared, then permissions should be set for group: users
-func checkWorkspaceRootPermissions(b *bundle.Bundle) diag.Diagnostics {
+// isUsersGroupPermissionSet checks that top-level permissions set for bundle contain group_name: users with CAN_MANAGE permission.
+func isUsersGroupPermissionSet(b *bundle.Bundle) diag.Diagnostics {
 	var diags diag.Diagnostics
 
 	allUsers := false
 	for _, p := range b.Config.Permissions {
 		if p.GroupName == "users" && p.Level == CAN_MANAGE {
 			allUsers = true
+			break
 		}
 	}
 

bundle/permissions/workspace_root_test.go

@@ -70,7 +70,7 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) {
 		WorkspaceObjectType: "directories",
 	}).Return(nil, nil)
 
-	diags := bundle.Apply(context.Background(), b, ApplyWorkspaceRootPermissions())
+	diags := bundle.Apply(context.Background(), b, bundle.Seq(ValidateSharedRootPermissions(), ApplyWorkspaceRootPermissions()))
 	require.Empty(t, diags)
 }
 
@@ -106,7 +106,7 @@ func TestApplyWorkspaceRootPermissionsForShared(t *testing.T) {
 		WorkspaceObjectType: "directories",
 	}).Return(nil, nil)
 
-	diags := bundle.Apply(context.Background(), b, ApplyWorkspaceRootPermissions())
+	diags := bundle.Apply(context.Background(), b, bundle.Seq(ValidateSharedRootPermissions(), ApplyWorkspaceRootPermissions()))
 	require.Empty(t, diags)
 }
 
@@ -130,8 +130,20 @@ func TestApplyWorkspaceRootPermissionsForSharedError(t *testing.T) {
 
 	m := mocks.NewMockWorkspaceClient(t)
 	b.SetWorkpaceClient(m.WorkspaceClient)
+	workspaceApi := m.GetMockWorkspaceAPI()
+	workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Workspace/Shared/foo/bar").Return(&workspace.ObjectInfo{
+		ObjectId: 1234,
+	}, nil)
 
-	diags := bundle.Apply(context.Background(), b, ApplyWorkspaceRootPermissions())
+	workspaceApi.EXPECT().UpdatePermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
+		AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
+			{UserName: "foo@bar.com", PermissionLevel: "CAN_MANAGE"},
+		},
+		WorkspaceObjectId:   "1234",
+		WorkspaceObjectType: "directories",
+	}).Return(nil, nil)
+
+	diags := bundle.Apply(context.Background(), b, bundle.Seq(ValidateSharedRootPermissions(), ApplyWorkspaceRootPermissions()))
 	require.Len(t, diags, 1)
 	require.Equal(t, "the bundle root path /Workspace/Shared/foo/bar is writable by all workspace users", diags[0].Summary)
 	require.Equal(t, diag.Warning, diags[0].Severity)

bundle/phases/initialize.go

@@ -74,8 +74,11 @@ func Initialize() bundle.Mutator {
 
 			mutator.TranslatePaths(),
 			trampoline.WrapperWarning(),
+
+			permissions.ValidateSharedRootPermissions(),
 			permissions.ApplyBundlePermissions(),
 			permissions.FilterCurrentUser(),
+
 			metadata.AnnotateJobs(),
 			metadata.AnnotatePipelines(),
 			terraform.Initialize(),