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_setting_smtp.go

@@ -30,12 +30,6 @@ const (
 	SMTPAuthLogin SMTPAuthType = "login"
 )
 
-var validSMTPAuthType = map[SMTPAuthType]struct{}{
-	SMTPAuthNone:  {},
-	SMTPAuthPlain: {},
-	SMTPAuthLogin: {},
-}
-
 // AdminSMTPSetting represents a the SMTP settings in Terraform Enterprise.
 type AdminSMTPSetting struct {
 	ID       string       `jsonapi:"primary,smtp-settings"`
@@ -82,6 +76,7 @@ func (a *adminSMTPSettings) Update(ctx context.Context, options AdminSMTPSetting
 	if err := options.valid(); err != nil {
 		return nil, err
 	}
+
 	req, err := a.client.newRequest("PATCH", "admin/smtp-settings", &options)
 	if err != nil {
 		return nil, err
@@ -98,11 +93,21 @@ func (a *adminSMTPSettings) Update(ctx context.Context, options AdminSMTPSetting
 
 func (o AdminSMTPSettingsUpdateOptions) valid() error {
 	if validString((*string)(o.Auth)) {
-		_, isValidType := validSMTPAuthType[*o.Auth]
-		if !isValidType {
-			return ErrInvalidSMTPAuth
+		if err := validateAdminSettingSMTPAuth(*o.Auth); err != nil {
+			return err
 		}
 	}
 
 	return nil
 }
+
+func validateAdminSettingSMTPAuth(authVal SMTPAuthType) error {
+	switch authVal {
+	case SMTPAuthNone, SMTPAuthPlain, SMTPAuthLogin:
+		// do nothing
+	default:
+		return ErrInvalidSMTPAuth
+	}
+
+	return nil
+}

admin_setting_smtp_integration_test.go

@@ -35,16 +35,17 @@ func TestAdminSettings_SMTP_Update(t *testing.T) {
 	client := testClient(t)
 	ctx := context.Background()
 
-	enabled := false
+	enabled := true
+	disabled := false
 
 	t.Run("with Auth option defined", func(t *testing.T) {
 		smtpSettings, err := client.Admin.Settings.SMTP.Update(ctx, AdminSMTPSettingsUpdateOptions{
-			Enabled: Bool(enabled),
+			Enabled: Bool(disabled),
 			Auth:    SMTPAuthValue(SMTPAuthNone),
 		})
 
 		require.NoError(t, err)
-		assert.Equal(t, enabled, smtpSettings.Enabled)
+		assert.Equal(t, disabled, smtpSettings.Enabled)
 	})
 	t.Run("with no Auth option", func(t *testing.T) {
 		smtpSettings, err := client.Admin.Settings.SMTP.Update(ctx, AdminSMTPSettingsUpdateOptions{
@@ -55,4 +56,13 @@ func TestAdminSettings_SMTP_Update(t *testing.T) {
 		assert.Equal(t, SMTPAuthNone, smtpSettings.Auth)
 		assert.Equal(t, enabled, smtpSettings.Enabled)
 	})
+	t.Run("with invalid Auth option", func(t *testing.T) {
+		var SMTPAuthPlained SMTPAuthType = "plained"
+		_, err := client.Admin.Settings.SMTP.Update(ctx, AdminSMTPSettingsUpdateOptions{
+			Enabled: Bool(disabled),
+			Auth:    &SMTPAuthPlained,
+		})
+
+		assert.Equal(t, err, ErrInvalidSMTPAuth)
+	})
 }

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
+}