Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose GitHub team ID #153

Merged
merged 10 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions provider/cmd/pulumi-resource-pulumiservice/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@
"type": "string"
},
"name": {
"description": "The team name.",
"description": "The team's name. Required for \"pulumi\" teams.",
"type": "string"
},
"displayName": {
Expand All @@ -473,8 +473,12 @@
}
},
"organizationName": {
"description": "The organization's name.",
"description": "The name of the Pulumi organization the team belongs to.",
"type": "string"
},
"githubTeamId": {
"description": "The GitHub ID of the team to mirror. Must be in the same GitHub organization that the Pulumi org is backed by.",
"type": "number"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to also be listed as an input property, right?

The schema still marks the other input properties as required, do those need to be made optional now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The schema still marks the other input properties as required, do those need to be made optional now?

Updated so name is now optional, org and type are still needed to construct relevant URLs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we validate the various combinations in Check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pgavlin doesn't seem like it https://github.com/pulumi/pulumi-pulumiservice/blob/main/provider/pkg/provider/team.go#L99-L101

Most of this provider's checks seem to no-op like this. If it's worth doing while we're in here I can implement it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of this provider's checks seem to no-op like this

:(

If it's worth doing while we're in here I can implement it.

I think that it's worth doing in this case b/c there are different combinations of valid inputs depending on the team type.

}
},
"inputProperties": {
Expand All @@ -483,7 +487,7 @@
"type": "string"
},
"name": {
"description": "The team name.",
"description": "The team's name. Required for \"pulumi\" teams.",
"type": "string"
},
"displayName": {
Expand All @@ -502,11 +506,15 @@
}
},
"organizationName": {
"description": "The organization's name.",
"description": "The name of the Pulumi organization the team belongs to.",
"type": "string"
},
"githubTeamId": {
"description": "The GitHub ID of the team to mirror. Must be in the same GitHub organization that the Pulumi org is backed by.",
"type": "number"
}
},
"requiredInputs": ["name", "organizationName", "teamType"]
"requiredInputs": ["organizationName", "teamType"]
},
"pulumiservice:index:TeamAccessToken": {
"description": "The Pulumi Cloud allows users to create access tokens scoped to team. Team access tokens is a resource to create them and assign them to a team",
Expand Down
29 changes: 12 additions & 17 deletions provider/pkg/internal/pulumiapi/teams.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
Expand Down Expand Up @@ -46,6 +46,7 @@ type createTeamRequest struct {
Name string `json:"name"`
DisplayName string `json:"displayName"`
Description string `json:"description"`
GitHubTeamID int64 `json:"githubTeamID,omitempty"`
}

type updateTeamRequest struct {
Expand Down Expand Up @@ -86,7 +87,6 @@ func (c *Client) ListTeams(ctx context.Context, orgName string) ([]Team, error)

var teamArray Teams
_, err := c.do(ctx, http.MethodGet, apiUrl, nil, &teamArray)

if err != nil {
return nil, fmt.Errorf("failed to list teams for %q: %w", orgName, err)
}
Expand All @@ -113,22 +113,22 @@ func (c *Client) GetTeam(ctx context.Context, orgName string, teamName string) (
return &team, nil
}

func (c *Client) CreateTeam(ctx context.Context, orgName, teamName, teamType, displayName, description string) (*Team, error) {
func (c *Client) CreateTeam(ctx context.Context, orgName, teamName, teamType, displayName, description string, teamID int64) (*Team, error) {
teamtypeList := []string{"github", "pulumi"}
if !contains(teamtypeList, teamType) {
return nil, fmt.Errorf("teamtype must be one of %v, got %q", teamtypeList, teamType)
}

if len(orgName) == 0 {
return nil, errors.New("orgname must not be empty")
}

if len(teamName) == 0 {
if len(teamName) == 0 && teamType != "github" {
return nil, errors.New("teamname must not be empty")
}

if len(teamType) == 0 {
return nil, errors.New("teamtype must not be empty")
}

teamtypeList := []string{"github", "pulumi"}
if !contains(teamtypeList, teamType) {
return nil, fmt.Errorf("teamtype must be one of %v, got %q", teamtypeList, teamType)
if teamType == "github" && teamID == 0 {
return nil, errors.New("github teams require a githubTeamId")
}

apiPath := path.Join("orgs", orgName, "teams", teamType)
Expand All @@ -139,11 +139,11 @@ func (c *Client) CreateTeam(ctx context.Context, orgName, teamName, teamType, di
Name: teamName,
DisplayName: displayName,
Description: description,
GitHubTeamID: teamID,
}

var team Team
_, err := c.do(ctx, http.MethodPost, apiPath, createReq, &team)

if err != nil {
return nil, fmt.Errorf("failed to create team: %w", err)
}
Expand All @@ -168,15 +168,13 @@ func (c *Client) UpdateTeam(ctx context.Context, orgName, teamName, displayName,
}

_, err := c.do(ctx, "PATCH", apiPath, updateReq, nil)

if err != nil {
return fmt.Errorf("failed to update team: %w", err)
}
return nil
}

func (c *Client) DeleteTeam(ctx context.Context, orgName, teamName string) error {

if len(orgName) == 0 {
return errors.New("orgname must not be empty")
}
Expand Down Expand Up @@ -220,7 +218,6 @@ func (c *Client) updateTeamMembership(ctx context.Context, orgName, teamName, us
}

_, err := c.do(ctx, http.MethodPatch, apiPath, updateMembershipReq, nil)

if err != nil {
return fmt.Errorf("failed to update team membership: %w", err)
}
Expand Down Expand Up @@ -266,7 +263,6 @@ func (c *Client) AddStackPermission(ctx context.Context, stack StackName, teamNa
}

_, err := c.do(ctx, http.MethodPatch, apiPath, addStackPermissionRequest, nil)

if err != nil {
return fmt.Errorf("failed to add stack permission for team: %w", err)
}
Expand All @@ -289,7 +285,6 @@ func (c *Client) RemoveStackPermission(ctx context.Context, stack StackName, tea
}

_, err := c.do(ctx, http.MethodPatch, apiPath, removeStackPermissionRequest, nil)

if err != nil {
return fmt.Errorf("failed to remove stack permission for team: %w", err)
}
Expand Down
77 changes: 61 additions & 16 deletions provider/pkg/internal/pulumiapi/teams_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,41 +98,63 @@ func TestGetTeam(t *testing.T) {
func TestCreateTeam(t *testing.T) {
orgName := "an-organization"
teamName := "a-team"
teamType := "pulumi"
displayName := "A Team"
description := "The A Team"
team := Team{
Type: teamType,
Name: teamName,
DisplayName: displayName,
Description: description,
}
t.Run("Happy Path", func(t *testing.T) {
t.Run("Happy Path (pulumi team)", func(t *testing.T) {
expected := Team{
Type: "pulumi",
Name: teamName,
DisplayName: displayName,
Description: description,
}
c, cleanup := startTestServer(t, testServerConfig{
ExpectedReqMethod: http.MethodPost,
ExpectedReqPath: "/api/orgs/an-organization/teams/pulumi",
ExpectedReqBody: createTeamRequest{
Organization: orgName,
TeamType: teamType,
TeamType: "pulumi",
Name: teamName,
DisplayName: displayName,
Description: description,
},
ResponseBody: team,
ResponseBody: expected,
ResponseCode: 201,
})
defer cleanup()
actualTeam, err := c.CreateTeam(ctx, orgName, teamName, teamType, displayName, description)
actualTeam, err := c.CreateTeam(ctx, orgName, teamName, "pulumi", displayName, description, 0)
assert.NoError(t, err)
assert.Equal(t, team, *actualTeam)
assert.Equal(t, expected, *actualTeam)
})
t.Run("Error", func(t *testing.T) {
t.Run("Happy Path (github team)", func(t *testing.T) {
expected := Team{
Type: "github",
Name: teamName,
DisplayName: displayName,
Description: description,
}
c, cleanup := startTestServer(t, testServerConfig{
ExpectedReqMethod: http.MethodPost,
ExpectedReqPath: "/api/orgs/an-organization/teams/github",
ExpectedReqBody: createTeamRequest{
TeamType: "github",
Organization: orgName,
GitHubTeamID: 1,
},
ResponseBody: expected,
ResponseCode: 201,
})
defer cleanup()
actualTeam, err := c.CreateTeam(ctx, orgName, "", "github", "", "", 1)
assert.NoError(t, err)
assert.Equal(t, expected, *actualTeam)
})
t.Run("Error (pulumi team)", func(t *testing.T) {
c, cleanup := startTestServer(t, testServerConfig{
ExpectedReqMethod: http.MethodPost,
ExpectedReqPath: "/api/orgs/an-organization/teams/pulumi",
ExpectedReqBody: createTeamRequest{
Organization: orgName,
TeamType: teamType,
TeamType: "pulumi",
Name: teamName,
DisplayName: displayName,
Description: description,
Expand All @@ -143,10 +165,34 @@ func TestCreateTeam(t *testing.T) {
},
})
defer cleanup()
team, err := c.CreateTeam(ctx, orgName, teamName, teamType, displayName, description)
team, err := c.CreateTeam(ctx, orgName, teamName, "pulumi", displayName, description, 0)
assert.Nil(t, team, "team should be nil since error was returned")
assert.EqualError(t, err, "failed to create team: 401 API error: unauthorized")
})
t.Run("Error (github team missing ID)", func(t *testing.T) {
c, cleanup := startTestServer(t, testServerConfig{})
defer cleanup()
_, err := c.CreateTeam(ctx, orgName, "", "github", "", "", 0)
assert.EqualError(t, err, "github teams require a githubTeamId")
})
t.Run("Error (invalid team type)", func(t *testing.T) {
c, cleanup := startTestServer(t, testServerConfig{})
defer cleanup()
_, err := c.CreateTeam(ctx, orgName, "", "foo", "", "", 0)
assert.EqualError(t, err, "teamtype must be one of [github pulumi], got \"foo\"")
})
t.Run("Error (invalid team name for pulumi team)", func(t *testing.T) {
c, cleanup := startTestServer(t, testServerConfig{})
defer cleanup()
_, err := c.CreateTeam(ctx, orgName, "", "pulumi", "", "", 0)
assert.EqualError(t, err, "teamname must not be empty")
})
t.Run("Error (missing org name)", func(t *testing.T) {
c, cleanup := startTestServer(t, testServerConfig{})
defer cleanup()
_, err := c.CreateTeam(ctx, "", "", "pulumi", "", "", 0)
assert.EqualError(t, err, "orgname must not be empty")
})
}

func TestAddMemberToTeam(t *testing.T) {
Expand Down Expand Up @@ -214,7 +260,6 @@ func TestAddStackPermission(t *testing.T) {
})
}


func TestRemoveStackPermission(t *testing.T) {
teamName := "a-team"
stack := StackName{
Expand Down
48 changes: 42 additions & 6 deletions provider/pkg/provider/team.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type PulumiServiceTeamInput struct {
Description string
OrganizationName string
Members []string
GitHubTeamID int64
}

func (i *PulumiServiceTeamInput) ToPropertyMap() resource.PropertyMap {
Expand All @@ -45,6 +46,7 @@ func (i *PulumiServiceTeamInput) ToPropertyMap() resource.PropertyMap {
pm["description"] = resource.NewPropertyValue(i.Description)
pm["members"] = resource.NewPropertyValue(i.Members)
pm["organizationName"] = resource.NewPropertyValue(i.OrganizationName)
pm["githubTeamId"] = resource.NewPropertyValue(i.GitHubTeamID)
return pm
}

Expand Down Expand Up @@ -85,6 +87,10 @@ func ToPulumiServiceTeamInput(inputMap resource.PropertyMap) PulumiServiceTeamIn
input.OrganizationName = inputMap["organizationName"].StringValue()
}

if inputMap["githubTeamId"].HasValue() && inputMap["githubTeamId"].IsNumber() {
input.GitHubTeamID = int64(inputMap["githubTeamId"].NumberValue())
}

return input
}

Expand Down Expand Up @@ -138,7 +144,38 @@ func (t *PulumiServiceTeamResource) Diff(req *pulumirpc.DiffRequest) (*pulumirpc
}

func (t *PulumiServiceTeamResource) Read(req *pulumirpc.ReadRequest) (*pulumirpc.ReadResponse, error) {
return nil, errors.New("Read construct is not yet implemented")
ctx := context.Background()

orgName, teamName, err := splitSingleSlashString(req.Id)
if err != nil {
return nil, err
}

team, err := t.client.GetTeam(ctx, orgName, teamName)
if err != nil {
return nil, fmt.Errorf("failed to read Team (%q): %w", req.Id, err)
}
if team == nil {
return &pulumirpc.ReadResponse{}, nil
}
inputs := PulumiServiceTeamInput{
Description: team.Description,
DisplayName: team.DisplayName,
Name: team.Name,
Type: team.Type,
OrganizationName: orgName,
}
for _, m := range team.Members {
inputs.Members = append(inputs.Members, m.GithubLogin)
}
props, err := plugin.MarshalProperties(inputs.ToPropertyMap(), plugin.MarshalOptions{})
if err != nil {
return nil, fmt.Errorf("failed to marshal inputs to properties: %w", err)
}
return &pulumirpc.ReadResponse{
Id: req.Id,
Properties: props,
}, nil
}

func (t *PulumiServiceTeamResource) Update(req *pulumirpc.UpdateRequest) (*pulumirpc.UpdateResponse, error) {
Expand All @@ -164,7 +201,8 @@ func (t *PulumiServiceTeamResource) Update(req *pulumirpc.UpdateRequest) (*pulum
t.updateTeam(context.Background(), inputsChanged)
}

if !Equal(teamOld.Members, teamNew.Members) {
// github teams can't manage membership.
if !Equal(teamOld.Members, teamNew.Members) && teamNew.Type != "github" {
inputsChanged.Members = teamNew.Members
for _, usernameToDelete := range teamOld.Members {
if !InSlice(usernameToDelete, teamNew.Members) {
Expand Down Expand Up @@ -245,17 +283,16 @@ func (t *PulumiServiceTeamResource) updateTeam(ctx context.Context, input Pulumi
}

func (t *PulumiServiceTeamResource) createTeam(ctx context.Context, input PulumiServiceTeamInput) (*string, error) {
_, err := t.client.CreateTeam(ctx, input.OrganizationName, input.Name, input.Type, input.DisplayName, input.Description)
team, err := t.client.CreateTeam(ctx, input.OrganizationName, input.Name, input.Type, input.DisplayName, input.Description, input.GitHubTeamID)
if err != nil {
return nil, err
}

teamUrn := fmt.Sprintf("%s/%s", input.OrganizationName, input.Name)
teamUrn := fmt.Sprintf("%s/%s", input.OrganizationName, team.Name)
return &teamUrn, nil
}

func (t *PulumiServiceTeamResource) deleteFromTeam(ctx context.Context, orgName string, teamName string, userName string) error {

if len(orgName) == 0 {
return errors.New("orgname must not be empty")
}
Expand All @@ -269,7 +306,6 @@ func (t *PulumiServiceTeamResource) deleteFromTeam(ctx context.Context, orgName
}

return t.client.DeleteMemberFromTeam(ctx, orgName, teamName, userName)

}

func (t *PulumiServiceTeamResource) addToTeam(ctx context.Context, orgName string, teamName string, userName string) error {
Expand Down
Loading
Loading