Skip to content

Commit

Permalink
fix: team_access endpoint is a PUT (#170)
Browse files Browse the repository at this point in the history 
* fix: team_access endpoint is a PUT

* words

* add acceptance test for team access

* oops

* list

* oops

* bah

* fix

* fix delete

* maybe

* final
  • Loading branch information
@parkedwards
parkedwards authored May 4, 2024
1 parent 056df6a commit f55b609
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 30 deletions.
2 changes: 2 additions & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ local.tfvars

# Built artifacts
build/*

.envrc
2 changes: 1 addition & 1 deletion internal/api/workspace_access.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
type WorkspaceAccessClient interface {
Upsert(ctx context.Context, accessorType string, accessorID uuid.UUID, roleID uuid.UUID) (*WorkspaceAccess, error)
Get(ctx context.Context, accessorType string, accessID uuid.UUID) (*WorkspaceAccess, error)
Delete(ctx context.Context, accessorType string, accessID uuid.UUID) error
Delete(ctx context.Context, accessorType string, accessID uuid.UUID, accessorID uuid.UUID) error
}

// WorkspaceAccess is a representation of a workspace access.
Expand Down
99 changes: 83 additions & 16 deletions internal/client/workspace_access.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,29 +45,45 @@ func (c *Client) WorkspaceAccess(accountID uuid.UUID, workspaceID uuid.UUID) (ap

// Upsert creates or updates access to a workspace for various accessor types.
func (c *WorkspaceAccessClient) Upsert(ctx context.Context, accessorType string, accessorID uuid.UUID, roleID uuid.UUID) (*api.WorkspaceAccess, error) {
payload := api.WorkspaceAccessUpsert{
WorkspaceRoleID: roleID,
// NOTE: our access APIs can optionally take a single "access" payload
// or a list of them. In our case, we'll always pass a 1-item slice
payloads := []api.WorkspaceAccessUpsert{
{
WorkspaceRoleID: roleID,
},
}
var requestPath string

// NOTE: this is a quirk of our <entity>_access API at the moment
// where user_access and bot_access were originally set up as a POST.
//
// Semantically, they should be a PUT, which the newer team_access API is set up as.
// At a later point, we will migrate the user/bot API variants over to a PUT
// in a breaking change.
var requestMethod string

if accessorType == utils.User {
requestPath = fmt.Sprintf("%s/user_access/", c.routePrefix)
payload.UserID = &accessorID
payloads[0].UserID = &accessorID
requestMethod = http.MethodPost
}
if accessorType == utils.ServiceAccount {
requestPath = fmt.Sprintf("%s/bot_access/", c.routePrefix)
payload.BotID = &accessorID
payloads[0].BotID = &accessorID
requestMethod = http.MethodPost
}
if accessorType == utils.Team {
requestPath = fmt.Sprintf("%s/team_access/", c.routePrefix)
payload.TeamID = &accessorID
payloads[0].TeamID = &accessorID
requestMethod = http.MethodPut
}

var buf bytes.Buffer
if err := json.NewEncoder(&buf).Encode(&payload); err != nil {
if err := json.NewEncoder(&buf).Encode(&payloads); err != nil {
return nil, fmt.Errorf("failed to encode upsert payload data: %w", err)
}

req, err := http.NewRequestWithContext(ctx, http.MethodPost, requestPath, &buf)
req, err := http.NewRequestWithContext(ctx, requestMethod, requestPath, &buf)
if err != nil {
return nil, fmt.Errorf("error creating request: %w", err)
}
Expand All @@ -85,28 +101,44 @@ func (c *WorkspaceAccessClient) Upsert(ctx context.Context, accessorType string,
return nil, fmt.Errorf("status code %s, error=%s", resp.Status, errorBody)
}

var workspaceAccess api.WorkspaceAccess
if err := json.NewDecoder(resp.Body).Decode(&workspaceAccess); err != nil {
var workspaceAccesses []api.WorkspaceAccess
if err := json.NewDecoder(resp.Body).Decode(&workspaceAccesses); err != nil {
return nil, fmt.Errorf("failed to decode response: %w", err)
}

return &workspaceAccess, nil
return &workspaceAccesses[0], nil
}

// Get fetches workspace access for various accessor types via accessID.
func (c *WorkspaceAccessClient) Get(ctx context.Context, accessorType string, accessID uuid.UUID) (*api.WorkspaceAccess, error) {
var requestPath string
var requestMethod string

// NOTE: this is a quirk of our <entity>_access API at the moment
// where user_access and bot_access can be fetched individually by resource ID,
// whereas team_access resources must be fetched as a list, scoped to the workspace.
//
// Here, we'll key off of the `accessorType` to determine the correct API endpoint,
// and we'll conditionally handle unmarshalling the response to
// a single WorkspaceAccess (for user_access and bot_access)
// or a list of them (for team_access) -> filtering for the passed `accessID`.
if accessorType == utils.User {
// GET: /.../<workspace_access_id>
requestMethod = http.MethodGet
requestPath = fmt.Sprintf("%s/user_access/%s", c.routePrefix, accessID.String())
}
if accessorType == utils.ServiceAccount {
// GET: /.../<workspace_access_id>
requestMethod = http.MethodGet
requestPath = fmt.Sprintf("%s/bot_access/%s", c.routePrefix, accessID.String())
}
if accessorType == utils.Team {
requestPath = fmt.Sprintf("%s/team_access/%s", c.routePrefix, accessID.String())
// POST: /.../filter
requestMethod = http.MethodPost
requestPath = fmt.Sprintf("%s/team_access/filter", c.routePrefix)
}

req, err := http.NewRequestWithContext(ctx, http.MethodGet, requestPath, http.NoBody)
req, err := http.NewRequestWithContext(ctx, requestMethod, requestPath, http.NoBody)
if err != nil {
return nil, fmt.Errorf("error creating request: %w", err)
}
Expand All @@ -125,24 +157,59 @@ func (c *WorkspaceAccessClient) Get(ctx context.Context, accessorType string, ac
}

var workspaceAccess api.WorkspaceAccess
if err := json.NewDecoder(resp.Body).Decode(&workspaceAccess); err != nil {
return nil, fmt.Errorf("failed to decode response: %w", err)
var workspaceAccesses []api.WorkspaceAccess

// If this is a team_access resource, we'll expect a list of WorkspaceAccess objects
if accessorType == utils.Team {
if err := json.NewDecoder(resp.Body).Decode(&workspaceAccesses); err != nil {
return nil, fmt.Errorf("failed to decode response: %w", err)
}
}
for _, access := range workspaceAccesses {
if access.ID == accessID {
workspaceAccess = access

break
}
}

// Otherwise, we'll expect a single WorkspaceAccess object, fetched by `accessID`
if accessorType == utils.User || accessorType == utils.ServiceAccount {
if err := json.NewDecoder(resp.Body).Decode(&workspaceAccess); err != nil {
return nil, fmt.Errorf("failed to decode response: %w", err)
}
}

if workspaceAccess.ID == uuid.Nil {
return nil, fmt.Errorf("workspace access not found for accessID: %s", accessID.String())
}

return &workspaceAccess, nil
}

// DeleteUserAccess deletes a service account's workspace access via accessID.
func (c *WorkspaceAccessClient) Delete(ctx context.Context, accessorType string, accessID uuid.UUID) error {
func (c *WorkspaceAccessClient) Delete(ctx context.Context, accessorType string, accessID uuid.UUID, accessorID uuid.UUID) error {
var requestPath string

// NOTE: this is a quirk of our <entity>_access API at the moment
// where user_access and bot_access can be deleted individually by the `accessID`
// whereas team_access resources must be deleted by the `accessorID`, eg. the team_id
//
// Here, we'll key off of the `accessorType` to determine the correct URL parameter to use.
//
// DELETE: /.../<workspace_access_id>
if accessorType == utils.User {
requestPath = fmt.Sprintf("%s/user_access/%s", c.routePrefix, accessID.String())
}

// DELETE: /.../<workspace_access_id>
if accessorType == utils.ServiceAccount {
requestPath = fmt.Sprintf("%s/bot_access/%s", c.routePrefix, accessID.String())
}

// DELETE: /.../<team_id>
if accessorType == utils.Team {
requestPath = fmt.Sprintf("%s/team_access/%s", c.routePrefix, accessID.String())
requestPath = fmt.Sprintf("%s/team_access/%s", c.routePrefix, accessorID.String())
}

req, err := http.NewRequestWithContext(ctx, http.MethodDelete, requestPath, http.NoBody)
Expand Down
8 changes: 6 additions & 2 deletions internal/provider/resources/workspace_access.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (r *WorkspaceAccessResource) Schema(_ context.Context, _ resource.SchemaReq

// copyWorkspaceAccessToModel copies the API resource to the Terraform model.
// Note that api.WorkspaceAccess represents a combined model for all accessor types,
// meaning accessory-specific attributes like BotID and UserID will be conditionally nil
// meaning accessor-specific attributes like BotID and UserID will be conditionally nil
// depending on the accessor type.
func copyWorkspaceAccessToModel(access *api.WorkspaceAccess, model *WorkspaceAccessResourceModel) {
model.ID = types.StringValue(access.ID.String())
Expand All @@ -131,6 +131,9 @@ func copyWorkspaceAccessToModel(access *api.WorkspaceAccess, model *WorkspaceAcc
if access.UserID != nil {
model.AccessorID = customtypes.NewUUIDValue(*access.UserID)
}
if access.TeamID != nil {
model.AccessorID = customtypes.NewUUIDValue(*access.TeamID)
}
}

// Create will create the Workspace Access resource through the API and insert it into the State.
Expand Down Expand Up @@ -269,9 +272,10 @@ func (r *WorkspaceAccessResource) Delete(ctx context.Context, req resource.Delet
return
}

accessorID := state.AccessorID.ValueUUID()
accessorType := state.AccessorType.ValueString()

err = client.Delete(ctx, accessorType, accessID)
err = client.Delete(ctx, accessorType, accessID, accessorID)
if err != nil {
resp.Diagnostics.Append(helpers.ResourceClientErrorDiagnostic("Workspace Access", "delete", err))

Expand Down
116 changes: 105 additions & 11 deletions internal/provider/resources/workspace_access_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"github.com/prefecthq/terraform-provider-prefect/internal/utils"
)

const workspaceDatsourceName = "data.prefect_workspace.evergreen"

func fixtureAccWorkspaceAccessResourceForBot(botName string) string {
return fmt.Sprintf(`
data "prefect_workspace_role" "developer" {
Expand Down Expand Up @@ -56,7 +58,6 @@ resource "prefect_workspace_access" "bot_access" {
func TestAccResource_bot_workspace_access(t *testing.T) {
accessResourceName := "prefect_workspace_access.bot_access"
botResourceName := "prefect_service_account.bot"
workspaceDatsourceName := "data.prefect_workspace.evergreen"
developerRoleDatsourceName := "data.prefect_workspace_role.developer"
runnerRoleDatsourceName := "data.prefect_workspace_role.runner"

Expand All @@ -74,8 +75,8 @@ func TestAccResource_bot_workspace_access(t *testing.T) {
Config: fixtureAccWorkspaceAccessResourceForBot(randomName),
Check: resource.ComposeAggregateTestCheckFunc(
// Check creation + existence of the workspace access resource, with matching linked attributes
testAccCheckWorkspaceAccessExists(accessResourceName, workspaceDatsourceName, utils.ServiceAccount, &workspaceAccess),
testAccCheckWorkspaceAccessValuesForBot(&workspaceAccess, botResourceName, developerRoleDatsourceName),
testAccCheckWorkspaceAccessExists(utils.ServiceAccount, accessResourceName, workspaceDatsourceName, &workspaceAccess),
testAccCheckWorkspaceAccessValuesForAccessor(utils.ServiceAccount, &workspaceAccess, botResourceName, developerRoleDatsourceName),
resource.TestCheckResourceAttrPair(accessResourceName, "accessor_id", botResourceName, "id"),
resource.TestCheckResourceAttrPair(accessResourceName, "workspace_id", workspaceDatsourceName, "id"),
resource.TestCheckResourceAttrPair(accessResourceName, "workspace_role_id", developerRoleDatsourceName, "id"),
Expand All @@ -85,8 +86,8 @@ func TestAccResource_bot_workspace_access(t *testing.T) {
Config: fixtureAccWorkspaceAccessResourceUpdateForBot(randomName),
Check: resource.ComposeAggregateTestCheckFunc(
// Check updating the role of the workspace access resource, with matching linked attributes
testAccCheckWorkspaceAccessExists(accessResourceName, workspaceDatsourceName, utils.ServiceAccount, &workspaceAccess),
testAccCheckWorkspaceAccessValuesForBot(&workspaceAccess, botResourceName, runnerRoleDatsourceName),
testAccCheckWorkspaceAccessExists(utils.ServiceAccount, accessResourceName, workspaceDatsourceName, &workspaceAccess),
testAccCheckWorkspaceAccessValuesForAccessor(utils.ServiceAccount, &workspaceAccess, botResourceName, runnerRoleDatsourceName),
resource.TestCheckResourceAttrPair(accessResourceName, "accessor_id", botResourceName, "id"),
resource.TestCheckResourceAttrPair(accessResourceName, "workspace_id", workspaceDatsourceName, "id"),
resource.TestCheckResourceAttrPair(accessResourceName, "workspace_role_id", runnerRoleDatsourceName, "id"),
Expand All @@ -96,7 +97,86 @@ func TestAccResource_bot_workspace_access(t *testing.T) {
})
}

func testAccCheckWorkspaceAccessExists(accessResourceName string, workspaceDatasourceName string, accessorType string, access *api.WorkspaceAccess) resource.TestCheckFunc {
func fixtureAccWorkspaceAccessResourceForTeam() string {
return `
data "prefect_workspace_role" "viewer" {
name = "Viewer"
}
data "prefect_workspace" "evergreen" {
handle = "github-ci-tests"
}
data "prefect_team" "my_team" {
name = "my-team"
}
resource "prefect_workspace_access" "team_access" {
accessor_type = "TEAM"
accessor_id = data.prefect_team.my_team.id
workspace_id = data.prefect_workspace.evergreen.id
workspace_role_id = data.prefect_workspace_role.viewer.id
}`
}

func fixtureAccWorkspaceAccessResourceUpdateForTeam() string {
return `
data "prefect_workspace_role" "runner" {
name = "Runner"
}
data "prefect_workspace" "evergreen" {
handle = "github-ci-tests"
}
data "prefect_team" "my_team" {
name = "my-team"
}
resource "prefect_workspace_access" "team_access" {
accessor_type = "TEAM"
accessor_id = data.prefect_team.my_team.id
workspace_id = data.prefect_workspace.evergreen.id
workspace_role_id = data.prefect_workspace_role.runner.id
}`
}

//nolint:paralleltest // we use the resource.ParallelTest helper instead
func TestAccResource_team_workspace_access(t *testing.T) {
accessResourceName := "prefect_workspace_access.team_access"
teamResourceName := "data.prefect_team.my_team"
viewerRoleDatsourceName := "data.prefect_workspace_role.viewer"
runnerRoleDatsourceName := "data.prefect_workspace_role.runner"

// We use this variable to store the fetched resource from the API
// and it will be shared between TestSteps via a pointer.
var workspaceAccess api.WorkspaceAccess

resource.ParallelTest(t, resource.TestCase{
ProtoV6ProviderFactories: testutils.TestAccProtoV6ProviderFactories,
PreCheck: func() { testutils.AccTestPreCheck(t) },
Steps: []resource.TestStep{
{
Config: fixtureAccWorkspaceAccessResourceForTeam(),
Check: resource.ComposeAggregateTestCheckFunc(
// Check creation + existence of the workspace access resource, with matching linked attributes
testAccCheckWorkspaceAccessExists(utils.Team, accessResourceName, workspaceDatsourceName, &workspaceAccess),
testAccCheckWorkspaceAccessValuesForAccessor(utils.Team, &workspaceAccess, teamResourceName, viewerRoleDatsourceName),
resource.TestCheckResourceAttrPair(accessResourceName, "accessor_id", teamResourceName, "id"),
resource.TestCheckResourceAttrPair(accessResourceName, "workspace_id", workspaceDatsourceName, "id"),
resource.TestCheckResourceAttrPair(accessResourceName, "workspace_role_id", viewerRoleDatsourceName, "id"),
),
},
{
Config: fixtureAccWorkspaceAccessResourceUpdateForTeam(),
Check: resource.ComposeAggregateTestCheckFunc(
// Check updating the role of the workspace access resource, with matching linked attributes
testAccCheckWorkspaceAccessExists(utils.Team, accessResourceName, workspaceDatsourceName, &workspaceAccess),
testAccCheckWorkspaceAccessValuesForAccessor(utils.Team, &workspaceAccess, teamResourceName, runnerRoleDatsourceName),
resource.TestCheckResourceAttrPair(accessResourceName, "accessor_id", teamResourceName, "id"),
resource.TestCheckResourceAttrPair(accessResourceName, "workspace_id", workspaceDatsourceName, "id"),
resource.TestCheckResourceAttrPair(accessResourceName, "workspace_role_id", runnerRoleDatsourceName, "id"),
),
},
},
})
}

func testAccCheckWorkspaceAccessExists(accessorType string, accessResourceName string, workspaceDatasourceName string, access *api.WorkspaceAccess) resource.TestCheckFunc {
return func(state *terraform.State) error {
workspaceAccessResource, exists := state.RootModule().Resources[accessResourceName]
if !exists {
Expand All @@ -116,6 +196,7 @@ func testAccCheckWorkspaceAccessExists(accessResourceName string, workspaceDatas
workspaceAccessClient, _ := c.WorkspaceAccess(uuid.Nil, workspaceID)

fetchedWorkspaceAccess, err := workspaceAccessClient.Get(context.Background(), accessorType, workspaceAccessID)

if err != nil {
return fmt.Errorf("Error fetching Workspace Access: %w", err)
}
Expand All @@ -129,15 +210,28 @@ func testAccCheckWorkspaceAccessExists(accessResourceName string, workspaceDatas
}
}

func testAccCheckWorkspaceAccessValuesForBot(fetchedAccess *api.WorkspaceAccess, botResourceName string, roleDatasourceName string) resource.TestCheckFunc {
func testAccCheckWorkspaceAccessValuesForAccessor(accessorType string, fetchedAccess *api.WorkspaceAccess, accessorResourceName string, roleDatasourceName string) resource.TestCheckFunc {
return func(state *terraform.State) error {
bot, exists := state.RootModule().Resources[botResourceName]
accessor, exists := state.RootModule().Resources[accessorResourceName]
if !exists {
return fmt.Errorf("Resource not found in state: %s", botResourceName)
return fmt.Errorf("Resource not found in state: %s", accessorResourceName)
}

if fetchedAccess.BotID.String() != bot.Primary.ID {
return fmt.Errorf("Expected Workspace Access BotID to be %s, got %s", bot.Primary.ID, fetchedAccess.BotID.String())
switch accessorType {
case utils.User:
if fetchedAccess.UserID.String() != accessor.Primary.ID {
return fmt.Errorf("Expected Workspace Access UserID to be %s, got %s", accessor.Primary.ID, fetchedAccess.UserID.String())
}
case utils.ServiceAccount:
if fetchedAccess.BotID.String() != accessor.Primary.ID {
return fmt.Errorf("Expected Workspace Access BotID to be %s, got %s", accessor.Primary.ID, fetchedAccess.BotID.String())
}
case utils.Team:
if fetchedAccess.TeamID.String() != accessor.Primary.ID {
return fmt.Errorf("Expected Workspace Access TeamID to be %s, got %s", accessor.Primary.ID, fetchedAccess.TeamID.String())
}
default:
return fmt.Errorf("Unsupported accessor type: %s", accessorType)
}

role, exists := state.RootModule().Resources[roleDatasourceName]
Expand Down

0 comments on commit f55b609

Please sign in to comment.