From e63d5a7ee041388335b9ec90e686370efca5731a Mon Sep 17 00:00:00 2001 From: Ilia Medvedev Date: Wed, 10 Jul 2024 15:35:12 +0300 Subject: [PATCH 1/2] fix related resource ignored in permissions --- codefresh/cfclient/permission.go | 4 ++-- codefresh/cfclient/user.go | 6 +++-- codefresh/data_account_gitops_settings.go | 20 ++++++++-------- codefresh/resource_account_gitops_settings.go | 4 ++-- codefresh/resource_permission.go | 24 ++++++++++++++----- codefresh/resource_permission_test.go | 12 ++++++++++ 6 files changed, 48 insertions(+), 22 deletions(-) diff --git a/codefresh/cfclient/permission.go b/codefresh/cfclient/permission.go index 181fc8d..27ca0e9 100644 --- a/codefresh/cfclient/permission.go +++ b/codefresh/cfclient/permission.go @@ -9,7 +9,7 @@ type Permission struct { ID string `json:"id,omitempty"` Team string `json:"role,omitempty"` Resource string `json:"resource,omitempty"` - RelatedResource string `json:"related_resource,omitempty"` + RelatedResource string `json:"relatedResource,omitempty"` Action string `json:"action,omitempty"` Account string `json:"account,omitempty"` Tags []string `json:"attributes,omitempty"` @@ -20,7 +20,7 @@ type NewPermission struct { ID string `json:"_id,omitempty"` Team string `json:"team,omitempty"` Resource string `json:"resource,omitempty"` - RelatedResource string `json:"related_resource,omitempty"` + RelatedResource string `json:"relatedResource,omitempty"` Action string `json:"action,omitempty"` Account string `json:"account,omitempty"` Tags []string `json:"tags,omitempty"` diff --git a/codefresh/cfclient/user.go b/codefresh/cfclient/user.go index 37d4769..ec42b52 100644 --- a/codefresh/cfclient/user.go +++ b/codefresh/cfclient/user.go @@ -228,7 +228,9 @@ func (client *Client) GetAllUsers() (*[]User, error) { var allUsers []User for !bIsDone { - var userPaginatedResp struct{Docs []User `json:"docs"`} + var userPaginatedResp struct { + Docs []User `json:"docs"` + } opts := RequestOptions{ Path: fmt.Sprintf("/admin/user?limit=%d&page=%d", limitPerQuery, nPageIndex), @@ -248,7 +250,7 @@ func (client *Client) GetAllUsers() (*[]User, error) { } if len(userPaginatedResp.Docs) > 0 { - allUsers = append(allUsers,userPaginatedResp.Docs...) + allUsers = append(allUsers, userPaginatedResp.Docs...) nPageIndex++ } else { bIsDone = true diff --git a/codefresh/data_account_gitops_settings.go b/codefresh/data_account_gitops_settings.go index 4245a58..7875786 100644 --- a/codefresh/data_account_gitops_settings.go +++ b/codefresh/data_account_gitops_settings.go @@ -13,28 +13,28 @@ func dataSourceAccountGitopsSettings() *schema.Resource { Read: dataSourceAccountGitopsSettingsRead, Schema: map[string]*schema.Schema{ "id": { - Type: schema.TypeString, + Type: schema.TypeString, Description: "Account Id", - Computed: true, + Computed: true, }, "name": { - Type: schema.TypeString, - Computed: true, + Type: schema.TypeString, + Computed: true, Description: "Account name for active account", }, "git_provider": { - Type: schema.TypeString, - Computed: true, + Type: schema.TypeString, + Computed: true, Description: "Git provider name", }, "git_provider_api_url": { - Type: schema.TypeString, - Computed: true, + Type: schema.TypeString, + Computed: true, Description: "Git provider API url", }, "shared_config_repository": { - Type: schema.TypeString, - Computed: true, + Type: schema.TypeString, + Computed: true, Description: "Shared config repository url", }, "admins": { diff --git a/codefresh/resource_account_gitops_settings.go b/codefresh/resource_account_gitops_settings.go index 81dd852..49e9f8b 100644 --- a/codefresh/resource_account_gitops_settings.go +++ b/codefresh/resource_account_gitops_settings.go @@ -24,9 +24,9 @@ func resourceAccountGitopsSettings() *schema.Resource { Delete: resourceAccountGitopsSettingsDelete, Schema: map[string]*schema.Schema{ "id": { - Type: schema.TypeString, + Type: schema.TypeString, Description: "Account Id", - Computed: true, + Computed: true, }, "name": { Type: schema.TypeString, diff --git a/codefresh/resource_permission.go b/codefresh/resource_permission.go index 1e9f137..63039ed 100644 --- a/codefresh/resource_permission.go +++ b/codefresh/resource_permission.go @@ -7,6 +7,7 @@ import ( "github.com/codefresh-io/terraform-provider-codefresh/codefresh/cfclient" "github.com/codefresh-io/terraform-provider-codefresh/codefresh/internal/datautil" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" funk "github.com/thoas/go-funk" @@ -96,7 +97,12 @@ The tags for which to apply the permission. Supports two custom tags: }, }, }, - CustomizeDiff: resourcePermissionCustomDiff, + CustomizeDiff: customdiff.All( + resourcePermissionCustomDiff, + customdiff.ForceNewIfChange("related_resource", func(ctx context.Context, oldValue, newValue, meta interface{}) bool { + return true + }), + ), } } @@ -206,6 +212,11 @@ func mapPermissionToResource(permission *cfclient.Permission, d *schema.Resource return err } + err = d.Set("related_resource", permission.RelatedResource) + if err != nil { + return err + } + err = d.Set("tags", permission.Tags) if err != nil { return err @@ -224,11 +235,12 @@ func mapResourceToPermission(d *schema.ResourceData) *cfclient.Permission { tags = []string{"*", "untagged"} } permission := &cfclient.Permission{ - ID: d.Id(), - Team: d.Get("team").(string), - Action: d.Get("action").(string), - Resource: d.Get("resource").(string), - Tags: tags, + ID: d.Id(), + Team: d.Get("team").(string), + Action: d.Get("action").(string), + Resource: d.Get("resource").(string), + RelatedResource: d.Get("related_resource").(string), + Tags: tags, } return permission diff --git a/codefresh/resource_permission_test.go b/codefresh/resource_permission_test.go index 5dd356e..f2b9d16 100644 --- a/codefresh/resource_permission_test.go +++ b/codefresh/resource_permission_test.go @@ -27,6 +27,18 @@ func TestAccCodefreshPermissionConfig(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "action", "create"), resource.TestCheckResourceAttr(resourceName, "resource", "pipeline"), resource.TestCheckResourceAttr(resourceName, "tags.0", "*"), + resource.TestCheckResourceAttr(resourceName, "related_resource",""), + resource.TestCheckResourceAttr(resourceName, "tags.1", "production"), + ), + }, + { + Config: testAccCodefreshPermissionConfig("create", "pipeline", "project", []string{"production", "*"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckCodefreshPermissionExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "action", "create"), + resource.TestCheckResourceAttr(resourceName, "resource", "pipeline"), + resource.TestCheckResourceAttr(resourceName, "related_resource", "project"), + resource.TestCheckResourceAttr(resourceName, "tags.0", "*"), resource.TestCheckResourceAttr(resourceName, "tags.1", "production"), ), }, From 0716c8b6d2f4f6f637cf3794fd4bd9035ad7abf9 Mon Sep 17 00:00:00 2001 From: Ilia Medvedev Date: Wed, 10 Jul 2024 18:10:19 +0300 Subject: [PATCH 2/2] fix resource update --- codefresh/cfclient/permission.go | 21 +++++++++++++++++ codefresh/resource_permission.go | 33 +++++++++++++++++---------- codefresh/resource_permission_test.go | 2 +- 3 files changed, 43 insertions(+), 13 deletions(-) diff --git a/codefresh/cfclient/permission.go b/codefresh/cfclient/permission.go index 27ca0e9..aaeea94 100644 --- a/codefresh/cfclient/permission.go +++ b/codefresh/cfclient/permission.go @@ -142,3 +142,24 @@ func (client *Client) DeletePermission(id string) error { return nil } + +func (client *Client) UpdatePermissionTags(permission *Permission) error { + + fullPath := fmt.Sprintf("/abac/tags/rule/%s", permission.ID) + + body, _ := EncodeToJSON(permission.Tags) + + opts := RequestOptions{ + Path: fullPath, + Method: "POST", + Body: body, + } + + _, err := client.RequestAPI(&opts) + + if err != nil { + return err + } + + return nil +} diff --git a/codefresh/resource_permission.go b/codefresh/resource_permission.go index 63039ed..b6b4c49 100644 --- a/codefresh/resource_permission.go +++ b/codefresh/resource_permission.go @@ -99,9 +99,6 @@ The tags for which to apply the permission. Supports two custom tags: }, CustomizeDiff: customdiff.All( resourcePermissionCustomDiff, - customdiff.ForceNewIfChange("related_resource", func(ctx context.Context, oldValue, newValue, meta interface{}) bool { - return true - }), ), } } @@ -163,18 +160,30 @@ func resourcePermissionRead(d *schema.ResourceData, meta interface{}) error { func resourcePermissionUpdate(d *schema.ResourceData, meta interface{}) error { client := meta.(*cfclient.Client) - permission := *mapResourceToPermission(d) - resp, err := client.CreatePermission(&permission) - if err != nil { - return err - } - deleteErr := resourcePermissionDelete(d, meta) - if deleteErr != nil { - log.Printf("[WARN] failed to delete permission %v: %v", permission, deleteErr) + // In case team, action or relatedResource or resource have changed - a new permission needs to be created (but without recreating the terraform resource as destruction of resources is alarming for end users) + if d.HasChanges("team", "action", "related_resource", "resource") { + deleteErr := resourcePermissionDelete(d, meta) + + if deleteErr != nil { + log.Printf("[WARN] failed to delete permission %v: %v", permission, deleteErr) + } + + resp, err := client.CreatePermission(&permission) + + if err != nil { + return err + } + + d.SetId(resp.ID) + // Only tags can be updated + } else if d.HasChange("tags") { + err := client.UpdatePermissionTags(&permission) + if err != nil { + return err + } } - d.SetId(resp.ID) return resourcePermissionRead(d, meta) } diff --git a/codefresh/resource_permission_test.go b/codefresh/resource_permission_test.go index f2b9d16..0b4ec91 100644 --- a/codefresh/resource_permission_test.go +++ b/codefresh/resource_permission_test.go @@ -27,7 +27,7 @@ func TestAccCodefreshPermissionConfig(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "action", "create"), resource.TestCheckResourceAttr(resourceName, "resource", "pipeline"), resource.TestCheckResourceAttr(resourceName, "tags.0", "*"), - resource.TestCheckResourceAttr(resourceName, "related_resource",""), + resource.TestCheckResourceAttr(resourceName, "related_resource", ""), resource.TestCheckResourceAttr(resourceName, "tags.1", "production"), ), },