add validation check for "include" typed string values

revert two changes made on PR #339

admin_organization.go

@@ -76,9 +76,7 @@ type AdminOrganizationList struct {
 // https://www.terraform.io/docs/cloud/api/admin/organizations.html#available-related-resources
 type AdminOrgIncludeOpt string
 
-const (
-	AdminOrgOwners AdminOrgIncludeOpt = "owners"
-)
+const AdminOrgOwners AdminOrgIncludeOpt = "owners"
 
 // AdminOrganizationListOptions represents the options for listing organizations via Admin API.
 type AdminOrganizationListOptions struct {
@@ -103,6 +101,9 @@ type AdminOrganizationID struct {
 
 // List all the organizations visible to the current user.
 func (s *adminOrganizations) List(ctx context.Context, options *AdminOrganizationListOptions) (*AdminOrganizationList, error) {
+	if err := options.valid(); err != nil {
+		return nil, err
+	}
 	u := "admin/organizations"
 	req, err := s.client.newRequest("GET", u, options)
 	if err != nil {
@@ -225,3 +226,28 @@ func (s *adminOrganizations) Delete(ctx context.Context, organization string) er
 
 	return s.client.do(ctx, req, nil)
 }
+
+func (o *AdminOrganizationListOptions) valid() error {
+	if o == nil {
+		return nil // nothing to validate
+	}
+
+	if err := validateAdminOrgIncludeParams(o.Include); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func validateAdminOrgIncludeParams(params []AdminOrgIncludeOpt) error {
+	for _, p := range params {
+		switch p {
+		case AdminOrgOwners:
+			// do nothing
+		default:
+			return ErrInvalidIncludeValue
+		}
+	}
+
+	return nil
+}

admin_run.go

@@ -116,39 +116,48 @@ func (s *adminRuns) ForceCancel(ctx context.Context, runID string, options Admin
 }
 
 func (o *AdminRunsListOptions) valid() error {
-	if o == nil { // no need to validate fields
+	if o == nil { // nothing to validate
 		return nil
 	}
-	if validString(&o.RunStatus) {
-		validRunStatus := map[string]int{
-			string(RunApplied):            1,
-			string(RunApplyQueued):        1,
-			string(RunApplying):           1,
-			string(RunCanceled):           1,
-			string(RunConfirmed):          1,
-			string(RunCostEstimated):      1,
-			string(RunCostEstimating):     1,
-			string(RunDiscarded):          1,
-			string(RunErrored):            1,
-			string(RunPending):            1,
-			string(RunPlanQueued):         1,
-			string(RunPlanned):            1,
-			string(RunPlannedAndFinished): 1,
-			string(RunPlanning):           1,
-			string(RunPolicyChecked):      1,
-			string(RunPolicyChecking):     1,
-			string(RunPolicyOverride):     1,
-			string(RunPolicySoftFailed):   1,
-		}
-		runStatus := strings.Split(o.RunStatus, ",")
 
+	if err := validateAdminRunFilterParams(o.RunStatus); err != nil {
+		return err
+	}
+
+	if err := validateAdminRunIncludeParams(o.Include); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func validateAdminRunFilterParams(runStatus string) error {
+	// For the platform, an invalid filter value is a semantically understood query that returns an empty set, no error, no warning. But for go-tfe, an invalid value is good enough reason to error prior to a network call to the platform:
+	if validString(&runStatus) {
+		runStatuses := strings.Split(runStatus, ",")
 		// iterate over our statuses, and ensure it is valid.
-		for _, status := range runStatus {
-			if _, present := validRunStatus[status]; !present {
-				return fmt.Errorf("invalid value %s for run status", status)
+		for _, status := range runStatuses {
+			switch status {
+			case string(RunApplied), string(RunApplyQueued), string(RunApplying), string(RunCanceled), string(RunConfirmed), string(RunCostEstimate), string(RunCostEstimating), string(RunDiscarded), string(RunErrored), string(RunPending), string(RunPlanQueued), string(RunPlanned), string(RunPlannedAndFinished), string(RunPlanning), string(RunPolicyChecked), string(RunPolicyChecking), string(RunPolicyOverride), string(RunPolicySoftFailed):
+				// do nothing
+			default:
+				return fmt.Errorf(`invalid value "%s" for run status`, status)
 			}
 		}
 	}
 
 	return nil
 }
+
+func validateAdminRunIncludeParams(params []AdminRunIncludeOpt) error {
+	for _, p := range params {
+		switch p {
+		case AdminRunWorkspace, AdminRunWorkspaceOrg, AdminRunWorkspaceOrgOwners:
+			// do nothing
+		default:
+			return ErrInvalidIncludeValue
+		}
+	}
+
+	return nil
+}

admin_run_integration_test.go

@@ -92,6 +92,14 @@ func TestAdminRuns_List(t *testing.T) {
 		assert.NotEmpty(t, rl.Items[0].Workspace.Organization.Name)
 	})
 
+	t.Run("with invalid Include option", func(t *testing.T) {
+		_, err := client.Admin.Runs.List(ctx, &AdminRunsListOptions{
+			Include: []AdminRunIncludeOpt{"workpsace"},
+		})
+
+		assert.Equal(t, err, ErrInvalidIncludeValue)
+	})
+
 	t.Run("with RunStatus.pending filter", func(t *testing.T) {
 		r1, err := client.Runs.Read(ctx, rTest1.ID)
 		assert.NoError(t, err)

admin_user.go

@@ -68,9 +68,7 @@ type AdminUserList struct {
 // https://www.terraform.io/docs/cloud/api/admin/users.html#available-related-resources
 type AdminUserIncludeOpt string
 
-const (
-	AdminUserOrgs AdminUserIncludeOpt = "organizations"
-)
+const AdminUserOrgs AdminUserIncludeOpt = "organizations"
 
 // AdminUserListOptions represents the options for listing users.
 // https://www.terraform.io/docs/cloud/api/admin/users.html#query-parameters
@@ -93,6 +91,10 @@ type AdminUserListOptions struct {
 
 // List all user accounts in the Terraform Enterprise installation
 func (a *adminUsers) List(ctx context.Context, options *AdminUserListOptions) (*AdminUserList, error) {
+	if err := options.valid(); err != nil {
+		return nil, err
+	}
+
 	u := "admin/users"
 	req, err := a.client.newRequest("GET", u, options)
 	if err != nil {
@@ -228,3 +230,27 @@ func (a *adminUsers) Disable2FA(ctx context.Context, userID string) (*AdminUser,
 
 	return au, nil
 }
+
+func (o *AdminUserListOptions) valid() error {
+	if o == nil {
+		return nil // nothing to validate
+	}
+
+	if err := validateAdminUserIncludeParams(o.Include); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func validateAdminUserIncludeParams(params []AdminUserIncludeOpt) error {
+	for _, p := range params {
+		switch p {
+		case AdminUserOrgs:
+			// do nothing
+		default:
+			return ErrInvalidIncludeValue
+		}
+	}
+	return nil
+}

admin_workspace.go

@@ -76,6 +76,10 @@ type AdminWorkspaceList struct {
 
 // List all the workspaces within a workspace.
 func (s *adminWorkspaces) List(ctx context.Context, options *AdminWorkspaceListOptions) (*AdminWorkspaceList, error) {
+	if err := options.valid(); err != nil {
+		return nil, err
+	}
+
 	u := "admin/workspaces"
 	req, err := s.client.newRequest("GET", u, options)
 	if err != nil {
@@ -126,3 +130,28 @@ func (s *adminWorkspaces) Delete(ctx context.Context, workspaceID string) error
 
 	return s.client.do(ctx, req, nil)
 }
+
+func (o *AdminWorkspaceListOptions) valid() error {
+	if o == nil {
+		return nil // nothing to validate
+	}
+
+	if err := validateAdminWSIncludeParams(o.Include); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func validateAdminWSIncludeParams(params []AdminWorkspaceIncludeOpt) error {
+	for _, p := range params {
+		switch p {
+		case AdminWorkspaceOrg, AdminWorkspaceCurrentRun, AdminWorkspaceOrgOwners:
+			// do nothing
+		default:
+			return ErrInvalidIncludeValue
+		}
+	}
+
+	return nil
+}

agent_pool.go

@@ -23,6 +23,9 @@ type AgentPools interface {
 	// Read a agent pool by its ID.
 	Read(ctx context.Context, agentPoolID string) (*AgentPool, error)
 
+	// Read a agent pool by its ID with the given options.
+	ReadWithOptions(ctx context.Context, agentPoolID string, options *AgentPoolReadOptions) (*AgentPool, error)
+
 	// Update an agent pool by its ID.
 	Update(ctx context.Context, agentPool string, options AgentPoolUpdateOptions) (*AgentPool, error)
 
@@ -48,18 +51,21 @@ type AgentPool struct {
 
 	// Relations
 	Organization *Organization `jsonapi:"relation,organization"`
+	Workspaces   []*Workspace  `jsonapi:"relation,workspaces"`
 }
 
 // A list of relations to include
 type AgentPoolIncludeOpt string
 
 const AgentPoolWorkspaces AgentPoolIncludeOpt = "workspaces"
 
+type AgentPoolReadOptions struct {
+	Include []AgentPoolIncludeOpt `url:"include,omitempty"`
+}
+
 // AgentPoolListOptions represents the options for listing agent pools.
 type AgentPoolListOptions struct {
 	ListOptions
-
-	Include []AgentPoolIncludeOpt `url:"include,omitempty"`
 }
 
 // AgentPoolCreateOptions represents the options for creating an agent pool.
@@ -120,11 +126,19 @@ func (s *agentPools) Create(ctx context.Context, organization string, options Ag
 	return pool, nil
 }
 
-// Read a single agent pool by its ID.
+// Read a single agent pool by its ID
 func (s *agentPools) Read(ctx context.Context, agentpoolID string) (*AgentPool, error) {
+	return s.ReadWithOptions(ctx, agentpoolID, nil)
+}
+
+// Read a single agent pool by its ID with options.
+func (s *agentPools) ReadWithOptions(ctx context.Context, agentpoolID string, options *AgentPoolReadOptions) (*AgentPool, error) {
 	if !validStringID(&agentpoolID) {
 		return nil, ErrInvalidAgentPoolID
 	}
+	if err := options.valid(); err != nil {
+		return nil, err
+	}
 
 	u := fmt.Sprintf("agent-pools/%s", url.QueryEscape(agentpoolID))
 	req, err := s.client.newRequest("GET", u, nil)
@@ -209,3 +223,27 @@ func (o AgentPoolUpdateOptions) valid() error {
 	}
 	return nil
 }
+
+func (o *AgentPoolReadOptions) valid() error {
+	if o == nil {
+		return nil // nothing to validate
+	}
+	if err := validateAgentPoolIncludeParams(o.Include); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func validateAgentPoolIncludeParams(params []AgentPoolIncludeOpt) error {
+	for _, p := range params {
+		switch p {
+		case AgentPoolWorkspaces:
+			// do nothing
+		default:
+			return ErrInvalidIncludeValue
+		}
+	}
+
+	return nil
+}

agent_pool_integration_test.go

@@ -49,14 +49,6 @@ func TestAgentPoolsList(t *testing.T) {
 		assert.Equal(t, 1, pools.TotalCount)
 	})
 
-	t.Run("with Include options", func(t *testing.T) {
-		pools, err := client.AgentPools.List(ctx, orgTest.Name, &AgentPoolListOptions{
-			Include: []AgentPoolIncludeOpt{AgentPoolWorkspaces},
-		})
-		require.NoError(t, err)
-		assert.NotEmpty(t, pools.Items[0].Organization.Name)
-	})
-
 	t.Run("without a valid organization", func(t *testing.T) {
 		pools, err := client.AgentPools.List(ctx, badIdentifier, nil)
 		assert.Nil(t, pools)
@@ -139,6 +131,21 @@ func TestAgentPoolsRead(t *testing.T) {
 		assert.Nil(t, k)
 		assert.EqualError(t, err, ErrInvalidAgentPoolID.Error())
 	})
+
+	t.Run("with Include option", func(t *testing.T) {
+		_, wTestCleanup := createWorkspaceWithVCS(t, client, orgTest, WorkspaceCreateOptions{
+			Name:          String("foo"),
+			ExecutionMode: String("agent"),
+			AgentPoolID:   String(pool.ID),
+		})
+		defer wTestCleanup()
+
+		k, err := client.AgentPools.ReadWithOptions(ctx, pool.ID, &AgentPoolReadOptions{
+			Include: []AgentPoolIncludeOpt{AgentPoolWorkspaces},
+		})
+		require.NoError(t, err)
+		assert.NotEmpty(t, k.Workspaces[0])
+	})
 }
 
 func TestAgentPoolsUpdate(t *testing.T) {