add validation check for include typed string values

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,23 @@ func (a *adminUsers) Disable2FA(ctx context.Context, userID string) (*AdminUser,
 
 	return au, nil
 }
+
+func (o *AdminUserListOptions) valid() error {
+	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,24 @@ func (s *adminWorkspaces) Delete(ctx context.Context, workspaceID string) error
 
 	return s.client.do(ctx, req, nil)
 }
+
+func (o *AdminWorkspaceListOptions) valid() error {
+	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

@@ -79,6 +79,9 @@ func (s *agentPools) List(ctx context.Context, organization string, options *Age
 	if !validStringID(&organization) {
 		return nil, ErrInvalidOrg
 	}
+	if err := options.valid(); err != nil {
+		return nil, err
+	}
 
 	u := fmt.Sprintf("organizations/%s/agent-pools", url.QueryEscape(organization))
 	req, err := s.client.newRequest("GET", u, options)
@@ -209,3 +212,24 @@ func (o AgentPoolUpdateOptions) valid() error {
 	}
 	return nil
 }
+
+func (o *AgentPoolListOptions) valid() error {
+	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
+}

configuration_version.go

@@ -98,20 +98,20 @@ type CVStatusTimestamps struct {
 	StartedAt  time.Time `jsonapi:"attr,started-at,rfc3339"`
 }
 
-// ConfigurationVersionIncludeOpt represents the available options for include query params.
+// ConfigVerIncludeOpt represents the available options for include query params.
 // https://www.terraform.io/docs/cloud/api/configuration-versions.html#available-related-resources
-type ConfigurationVersionIncludeOpt string
+type ConfigVerIncludeOpt string
 
 const (
-	ConfigurationVerIngressAttributes ConfigurationVersionIncludeOpt = "ingress_attributes"
-	ConfigurationRun                  ConfigurationVersionIncludeOpt = "run"
+	ConfigVerIngressAttributes ConfigVerIncludeOpt = "ingress_attributes"
+	ConfigVerRun               ConfigVerIncludeOpt = "run"
 )
 
 // ConfigurationVersionReadOptions represents the options for reading a configuration version.
 type ConfigurationVersionReadOptions struct {
 	// Optional: A list of relations to include. See available resources:
 	// https://www.terraform.io/docs/cloud/api/configuration-versions.html#available-related-resources
-	Include []ConfigurationVersionIncludeOpt `url:"include,omitempty"`
+	Include []ConfigVerIncludeOpt `url:"include,omitempty"`
 }
 
 // ConfigurationVersionListOptions represents the options for listing
@@ -120,7 +120,7 @@ type ConfigurationVersionListOptions struct {
 	ListOptions
 	// Optional: A list of relations to include. See available resources:
 	// https://www.terraform.io/docs/cloud/api/configuration-versions.html#available-related-resources
-	Include []ConfigurationVersionIncludeOpt `url:"include,omitempty"`
+	Include []ConfigVerIncludeOpt `url:"include,omitempty"`
 }
 
 // ConfigurationVersionCreateOptions represents the options for creating a
@@ -170,6 +170,9 @@ func (s *configurationVersions) List(ctx context.Context, workspaceID string, op
 	if !validStringID(&workspaceID) {
 		return nil, ErrInvalidWorkspaceID
 	}
+	if err := options.valid(); err != nil {
+		return nil, err
+	}
 
 	u := fmt.Sprintf("workspaces/%s/configuration-versions", url.QueryEscape(workspaceID))
 	req, err := s.client.newRequest("GET", u, options)
@@ -218,6 +221,9 @@ func (s *configurationVersions) ReadWithOptions(ctx context.Context, cvID string
 	if !validStringID(&cvID) {
 		return nil, ErrInvalidConfigVersionID
 	}
+	if err := options.valid(); err != nil {
+		return nil, err
+	}
 
 	u := fmt.Sprintf("configuration-versions/%s", url.QueryEscape(cvID))
 	req, err := s.client.newRequest("GET", u, options)
@@ -260,3 +266,32 @@ func (s *configurationVersions) Upload(ctx context.Context, u, path string) erro
 
 	return s.client.do(ctx, req, nil)
 }
+
+func (o *ConfigurationVersionReadOptions) valid() error {
+	if err := validateConfigVerIncludeParams(o.Include); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (o *ConfigurationVersionListOptions) valid() error {
+	if err := validateConfigVerIncludeParams(o.Include); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func validateConfigVerIncludeParams(params []ConfigVerIncludeOpt) error {
+	for _, p := range params {
+		switch p {
+		case ConfigVerIngressAttributes, ConfigVerRun:
+			// do nothing
+		default:
+			return ErrInvalidIncludeValue
+		}
+	}
+
+	return nil
+}

configuration_version_integration_test.go

@@ -173,7 +173,7 @@ func TestConfigurationVersionsReadWithOptions(t *testing.T) {
 
 	t.Run("when the configuration version exists", func(t *testing.T) {
 		options := &ConfigurationVersionReadOptions{
-			Include: []ConfigurationVersionIncludeOpt{ConfigurationVerIngressAttributes},
+			Include: []ConfigVerIncludeOpt{ConfigVerIngressAttributes},
 		}
 
 		cv, err := client.ConfigurationVersions.ReadWithOptions(ctx, cv.ID, options)