Feat: New ABAC Attributes (#114)

## What

* `codefresh_permission.resource` : `project` is a new possible value
* `codefresh_permission.related_resource` — new attribute; only possible
value is project
* overhaul validation in `codefresh_permission`
* Fix `ImportStateVerify` issue related to
hashicorp/terraform-provider-aws#27049

## Why

New ABAC behavior in CF API

## Notes
<!-- Add any notes here -->

## Checklist

* [x] _I have read
[CONTRIBUTING.md](https://github.com/codefresh-io/terraform-provider-codefresh/blob/master/README.md)._
* [x] _I have [allowed changes to my fork to be
made](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork)._
* [x] _I have added tests, assuming new tests are warranted_.
* [x] _I understand that the `/test` comment will be ignored by the CI
trigger [unless it is made by a repo admin or
collaborator](https://codefresh.io/docs/docs/pipelines/triggers/git-triggers/#support-for-building-pull-requests-from-forks)._

Author: korenyoni

client/permission.go

@@ -2,30 +2,30 @@ package client
 
 import (
 	"fmt"
-	//"log"
 )
 
 // Permission spec
 type Permission struct {
-	ID       string   `json:"id,omitempty"`
-	Team     string   `json:"role,omitempty"`
-	Resource string   `json:"resource,omitempty"`
-	Action   string   `json:"action,omitempty"`
-	Account  string   `json:"account,omitempty"`
-	Tags     []string `json:"attributes,omitempty"`
+	ID              string   `json:"id,omitempty"`
+	Team            string   `json:"role,omitempty"`
+	Resource        string   `json:"resource,omitempty"`
+	RelatedResource string   `json:"related_resource,omitempty"`
+	Action          string   `json:"action,omitempty"`
+	Account         string   `json:"account,omitempty"`
+	Tags            []string `json:"attributes,omitempty"`
 }
 
-// NewPermission spec, diffs from Permission is `json:"team,omitempty"` vs `json:"role,omitempty"`
+// NewPermission spec, diffs from Permission: `json:"_id,omitempty"`, `json:"team,omitempty"`, `json:"tags,omitempty"`
 type NewPermission struct {
-	ID       string   `json:"_id,omitempty"`
-	Team     string   `json:"team,omitempty"`
-	Resource string   `json:"resource,omitempty"`
-	Action   string   `json:"action,omitempty"`
-	Account  string   `json:"account,omitempty"`
-	Tags     []string `json:"tags,omitempty"`
+	ID              string   `json:"_id,omitempty"`
+	Team            string   `json:"team,omitempty"`
+	Resource        string   `json:"resource,omitempty"`
+	RelatedResource string   `json:"related_resource,omitempty"`
+	Action          string   `json:"action,omitempty"`
+	Account         string   `json:"account,omitempty"`
+	Tags            []string `json:"tags,omitempty"`
 }
 
-// GetPermissionList -
 func (client *Client) GetPermissionList(teamID, action, resource string) ([]Permission, error) {
 	fullPath := "/abac"
 	opts := RequestOptions{
@@ -84,16 +84,16 @@ func (client *Client) GetPermissionByID(id string) (*Permission, error) {
 	return &permission, nil
 }
 
-// CreatePermision -
 func (client *Client) CreatePermission(permission *Permission) (*Permission, error) {
 
 	newPermission := &NewPermission{
-		ID:       permission.ID,
-		Team:     permission.Team,
-		Resource: permission.Resource,
-		Action:   permission.Action,
-		Account:  permission.Account,
-		Tags:     permission.Tags,
+		ID:              permission.ID,
+		Team:            permission.Team,
+		Resource:        permission.Resource,
+		RelatedResource: permission.RelatedResource,
+		Action:          permission.Action,
+		Account:         permission.Account,
+		Tags:            permission.Tags,
 	}
 
 	body, err := EncodeToJSON(newPermission)
@@ -113,8 +113,6 @@ func (client *Client) CreatePermission(permission *Permission) (*Permission, err
 		return nil, err
 	}
 
-	// respStr := string(resp)
-	// log.Printf("[DEBUG] createPermission responce body = %s", respStr)
 	var permissionResp []Permission
 	err = DecodeResponseInto(resp, &permissionResp)
 	if err != nil {
@@ -129,7 +127,6 @@ func (client *Client) CreatePermission(permission *Permission) (*Permission, err
 	return client.GetPermissionByID(newPermissionID)
 }
 
-// DeletePermission -
 func (client *Client) DeletePermission(id string) error {
 	fullPath := fmt.Sprintf("/abac/%s", id)
 	opts := RequestOptions{

codefresh/resource_context_test.go

@@ -162,7 +162,7 @@ func testAccCheckCodefreshContextExists(resource string) resource.TestCheckFunc
 		_, err := apiClient.GetContext(contextID)
 
 		if err != nil {
-			return fmt.Errorf("error fetching context with resource %s. %s", resource, err)
+			return fmt.Errorf("error fetching context with ID %s. %s", contextID, err)
 		}
 		return nil
 	}

codefresh/resource_permission.go

@@ -1,11 +1,14 @@
 package codefresh
 
 import (
+	"context"
 	"fmt"
 	"log"
 
 	cfClient "github.com/codefresh-io/terraform-provider-codefresh/client"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
+	funk "github.com/thoas/go-funk"
 )
 
 func resourcePermission() *schema.Resource {
@@ -35,16 +38,26 @@ func resourcePermission() *schema.Resource {
 The type of resources the permission applies to. Possible values:
 	* pipeline
 	* cluster
+	* project
 				`,
 				Type:     schema.TypeString,
 				Required: true,
-				ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) {
-					v := val.(string)
-					if v != "cluster" && v != "pipeline" {
-						errs = append(errs, fmt.Errorf("%q must be between \"pipeline\" or \"cluster\", got: %s", key, v))
-					}
-					return
-				},
+				ValidateFunc: validation.StringInSlice([]string{
+					"pipeline",
+					"cluster",
+					"project",
+				}, false),
+			},
+			"related_resource": {
+				Description: `
+Specifies the resource to use when evaluating the tags. Possible values:
+	* project
+`,
+				Type:     schema.TypeString,
+				Optional: true,
+				ValidateFunc: validation.StringInSlice([]string{
+					"project",
+				}, false),
 			},
 			"action": {
 				Description: `
@@ -59,13 +72,15 @@ Action to be allowed. Possible values:
 				`,
 				Type:     schema.TypeString,
 				Required: true,
-				ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) {
-					v := val.(string)
-					if v != "create" && v != "read" && v != "update" && v != "delete" && v != "run" && v != "approve" && v != "debug" {
-						errs = append(errs, fmt.Errorf("%q must be between one of create,read,update,delete,approve,debug got: %s", key, v))
-					}
-					return
-				},
+				ValidateFunc: validation.StringInSlice([]string{
+					"create",
+					"read",
+					"update",
+					"delete",
+					"run",
+					"approve",
+					"debug",
+				}, false),
 			},
 			"tags": {
 				Description: `
@@ -80,9 +95,24 @@ The effective tags to apply the permission. It supports 2 custom tags:
 				},
 			},
 		},
+		CustomizeDiff: resourcePermissionCustomDiff,
 	}
 }
 
+func resourcePermissionCustomDiff(ctx context.Context, diff *schema.ResourceDiff, v interface{}) error {
+	if diff.HasChanges("resource", "related_resource") {
+		if diff.Get("related_resource").(string) != "" && diff.Get("resource").(string) != "pipeline" {
+			return fmt.Errorf("related_resource is only valid when resource is 'pipeline'")
+		}
+	}
+	if diff.HasChanges("resource", "action") {
+		if funk.Contains([]string{"run", "approve", "debug"}, diff.Get("action").(string)) && diff.Get("resource").(string) != "pipeline" {
+			return fmt.Errorf("action %v is only valid when resource is 'pipeline'", diff.Get("action").(string))
+		}
+	}
+	return nil
+}
+
 func resourcePermissionCreate(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*cfClient.Client)
 
@@ -128,7 +158,6 @@ func resourcePermissionUpdate(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*cfClient.Client)
 
 	permission := *mapResourceToPermission(d)
-	permission.ID = ""
 	resp, err := client.CreatePermission(&permission)
 	if err != nil {
 		return err

codefresh/resource_permission_test.go

@@ -0,0 +1,85 @@
+package codefresh
+
+import (
+	"fmt"
+	cfClient "github.com/codefresh-io/terraform-provider-codefresh/client"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	funk "github.com/thoas/go-funk"
+)
+
+func TestAccCodefreshPermissionConfig(t *testing.T) {
+	resourceName := "codefresh_permission.test"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckCodefreshContextDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccCodefreshPermissionConfig("create", "pipeline", "null", []string{"production", "*"}),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckCodefreshPermissionExists(resourceName),
+					resource.TestCheckResourceAttr(resourceName, "action", "create"),
+					resource.TestCheckResourceAttr(resourceName, "resource", "pipeline"),
+					resource.TestCheckResourceAttr(resourceName, "tags.0", "*"),
+					resource.TestCheckResourceAttr(resourceName, "tags.1", "production"),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+func testAccCheckCodefreshPermissionExists(resource string) resource.TestCheckFunc {
+	return func(state *terraform.State) error {
+		rs, ok := state.RootModule().Resources[resource]
+		if !ok {
+			return fmt.Errorf("Not found: %s", resource)
+		}
+		if rs.Primary.ID == "" {
+			return fmt.Errorf("No Record ID is set")
+		}
+
+		permissionID := rs.Primary.ID
+
+		apiClient := testAccProvider.Meta().(*cfClient.Client)
+		_, err := apiClient.GetPermissionByID(permissionID)
+
+		if err != nil {
+			return fmt.Errorf("error fetching permission with ID %s. %s", permissionID, err)
+		}
+		return nil
+	}
+}
+
+// CONFIGS
+func testAccCodefreshPermissionConfig(action, resource, relatedResource string, tags []string) string {
+	escapeString := func(str string) string {
+		if str == "null" {
+			return str // null means Terraform should ignore this field
+		}
+		return fmt.Sprintf(`"%s"`, str)
+	}
+	tagsEscaped := funk.Map(tags, escapeString).([]string)
+
+	return fmt.Sprintf(`
+	data "codefresh_team" "users" {
+		name = "users"
+	}
+
+	resource "codefresh_permission" "test" {
+		team             = data.codefresh_team.users.id
+		action           = %s
+		resource         = %s
+		related_resource = %s
+		tags             = [%s]
+	}
+`, escapeString(action), escapeString(resource), escapeString(relatedResource), strings.Join(tagsEscaped[:], ","))
+}

codefresh/resource_step_types.go

@@ -198,8 +198,9 @@ func resourceStepTypesUpdate(ctx context.Context, d *schema.ResourceData, meta i
 	}
 
 	// Parse current set: new versions that need to be created are added to a data structure
-	// that will be sorted later for the creation
-	// Updates are performed immediately
+	// that will be sorted later for the creation.
+	// Updates are performed immediately.
+	// Also, see notes in TestAccCodefreshStepTypes regarding `version` and d.GetId()
 	for _, version := range stepTypesVersions.Versions {
 		versionNumber := version.VersionNumber
 		versionsDefined[versionNumber] = versionNumber
@@ -215,7 +216,6 @@ func resourceStepTypesUpdate(ctx context.Context, d *schema.ResourceData, meta i
 			_, err := client.UpdateStepTypes(&version.StepTypes)
 			if err != nil {
 				return diag.Errorf("[DEBUG] Error while updating stepTypes. Error = %v", err)
-
 			}
 		}
 	}

codefresh/resource_step_types_test.go

@@ -143,6 +143,11 @@ func TestAccCodefreshStepTypes(t *testing.T) {
 					ResourceName:      resourceName,
 					ImportState:       true,
 					ImportStateVerify: true,
+					// `codefresH_step_types` cannot retrieve `version` on Read only using d.GetId(),
+					// hence ImportStateVerify will always retrieve an unequilvalent value
+					// for `version`. We ignore this field for the purpose of the test.
+					// See: https://developer.hashicorp.com/terraform/plugin/sdkv2/resources/import#importstateverifyignore-1
+					ImportStateVerifyIgnore: []string{"version"},
 				},
 			},
 		})

docs/resources/permission.md

@@ -51,11 +51,14 @@ resource "codefresh_permission" "developers" {
 - `resource` (String) The type of resources the permission applies to. Possible values:
 	* pipeline
 	* cluster
+	* project
 - `team` (String) The Id of the team the permissions apply to.
 
 ### Optional
 
 - `_id` (String) The permission ID.
+- `related_resource` (String) Specifies the resource to use when evaluating the tags. Possible values:
+	* project
 - `tags` (Set of String) The effective tags to apply the permission. It supports 2 custom tags:
 	* untagged is a “tag” which refers to all clusters that don't have any tag.
 	* (the star character) means all tags.

examples/permissions/main.tf

@@ -8,16 +8,25 @@ data "codefresh_team" "developers" {
 
 resource "codefresh_permission" "dev_pipeline" {
   for_each = toset(["run", "create", "update", "delete", "read"])
-  team = data.codefresh_team.developers.id
-  action = each.value
+  team     = data.codefresh_team.developers.id
+  action   = each.value
   resource = "pipeline"
-  tags = [ "dev", "untagged"]
+  tags     = ["dev", "untagged"]
 }
 
 resource "codefresh_permission" "admin_pipeline" {
   for_each = toset(["run", "create", "update", "delete", "read", "approve"])
-  team = data.codefresh_team.admins.id
-  action = each.value
+  team     = data.codefresh_team.admins.id
+  action   = each.value
   resource = "pipeline"
-  tags = [ "production", "*"]
+  tags     = ["production", "*"]
+}
+
+resource "codefresh_permission" "admin_pipeline_related_resource" {
+  for_each         = toset(["run", "create", "update", "delete", "read", "approve"])
+  team             = data.codefresh_team.admins.id
+  action           = each.value
+  resource         = "pipeline"
+  related_resource = "project"
+  tags             = ["production", "*"]
 }