From bce29ee5eb876ac568a5c0a4a6ccbe800bd9f839 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20D=C5=82ugoszek?= Date: Fri, 8 Nov 2024 07:35:17 +0100 Subject: [PATCH] feat: Implement querying for spaces using relative paths (#576) * feat: Querying spaces by relative paths * Fix docs * Fix docs * fix test * fix test * Update docs/data-sources/space_by_path.md Co-authored-by: Adam Connelly * Update docs/data-sources/space_by_path.md Co-authored-by: Adam Connelly * Add ambiguity test case * fix comments * change error message --------- Co-authored-by: Adam Connelly --- docs/data-sources/space_by_path.md | 7 ++ .../spacelift_space_by_path/data-source.tf | 7 +- spacelift/data_current_space.go | 31 +++++++-- spacelift/data_space_by_path.go | 34 ++++++++-- spacelift/data_space_by_path_test.go | 68 ++++++++++++++++--- 5 files changed, 122 insertions(+), 25 deletions(-) diff --git a/docs/data-sources/space_by_path.md b/docs/data-sources/space_by_path.md index 249cc193..182da847 100644 --- a/docs/data-sources/space_by_path.md +++ b/docs/data-sources/space_by_path.md @@ -4,6 +4,7 @@ page_title: "spacelift_space_by_path Data Source - terraform-provider-spacelift" subcategory: "" description: |- spacelift_space_by_path represents a Spacelift space - a collection of resources such as stacks, modules, policies, etc. Allows for more granular access control. Can have a parent space. In contrary to spacelift_space, this resource is identified by a path, not by an ID. For this data source to work, path must be unique. If there are multiple spaces with the same path, this datasource will fail. + This data source can be used either with absolute paths (starting with root) or relative paths. When using a relative path, the path is relative to the current run's space. Disclaimer: This datasource can only be used in a stack that resides in a space with inheritance enabled. In addition, the parent spaces (excluding root) must also have inheritance enabled. --- @@ -11,6 +12,7 @@ description: |- # spacelift_space_by_path (Data Source) `spacelift_space_by_path` represents a Spacelift **space** - a collection of resources such as stacks, modules, policies, etc. Allows for more granular access control. Can have a parent space. In contrary to `spacelift_space`, this resource is identified by a path, not by an ID. For this data source to work, path must be unique. If there are multiple spaces with the same path, this datasource will fail. +This data source can be used either with absolute paths (starting with root) or relative paths. When using a relative path, the path is relative to the current run's space. **Disclaimer:** This datasource can only be used in a stack that resides in a space with inheritance enabled. In addition, the parent spaces (excluding root) must also have inheritance enabled. @@ -24,6 +26,11 @@ data "spacelift_space_by_path" "space" { output "space_description" { value = data.spacelift_space_by_path.space.description } + +// The following example shows how to use a relative path. If this is used in a stack in the root space, this is identical to using a path of `root/second space/my space`. +data "spacelift_space_by_relative_path" "space" { + space_path = "second space/my space" +} ``` diff --git a/examples/data-sources/spacelift_space_by_path/data-source.tf b/examples/data-sources/spacelift_space_by_path/data-source.tf index 26328c2d..3b82fedd 100644 --- a/examples/data-sources/spacelift_space_by_path/data-source.tf +++ b/examples/data-sources/spacelift_space_by_path/data-source.tf @@ -4,4 +4,9 @@ data "spacelift_space_by_path" "space" { output "space_description" { value = data.spacelift_space_by_path.space.description -} \ No newline at end of file +} + +// The following example shows how to use a relative path. If this is used in a stack in the root space, this is identical to using a path of `root/second space/my space`. +data "spacelift_space_by_relative_path" "space" { + space_path = "second space/my space" +} diff --git a/spacelift/data_current_space.go b/spacelift/data_current_space.go index 4f85d9b1..ec556217 100644 --- a/spacelift/data_current_space.go +++ b/spacelift/data_current_space.go @@ -2,6 +2,7 @@ package spacelift import ( "context" + "fmt" "path" "strings" @@ -54,25 +55,28 @@ func dataCurrentSpace() *schema.Resource { } } -func dataCurrentSpaceRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func getStackIDFromToken(token string) (string, error) { var claims jwt.StandardClaims - _, _, err := (&jwt.Parser{}).ParseUnverified(meta.(*internal.Client).Token, &claims) + _, _, err := (&jwt.Parser{}).ParseUnverified(token, &claims) if err != nil { // Don't care about validation errors, we don't actually validate those // tokens, we only parse them. var unverifiable *jwt.UnverfiableTokenError if !errors.As(err, &unverifiable) { - return diag.Errorf("could not parse client token: %v", err) + return "", fmt.Errorf("could not parse client token: %v", err) } } if issuer := claims.Issuer; issuer != "spacelift" { - return diag.Errorf("unexpected token issuer %s, is this a Spacelift run?", issuer) + return "", fmt.Errorf("unexpected token issuer %s, is this a Spacelift run?", issuer) } stackID, _ := path.Split(claims.Subject) + return stackID, nil +} +func getSpaceForStack(ctx context.Context, stackID string, meta interface{}) (structs.Space, error) { var query struct { Stack *structs.Stack `graphql:"stack(id: $id)"` Module *structs.Module `graphql:"module(id: $id)"` @@ -81,9 +85,9 @@ func dataCurrentSpaceRead(ctx context.Context, d *schema.ResourceData, meta inte variables := map[string]interface{}{"id": toID(strings.TrimRight(stackID, "/"))} if err := meta.(*internal.Client).Query(ctx, "StackRead", &query, variables); err != nil { if strings.Contains(err.Error(), "denied") { - return diag.Errorf("could not query for stack: %v, is this stack administrative?", err) + return structs.Space{}, fmt.Errorf("could not query for stack: %v, is this stack administrative?", err) } - return diag.Errorf("could not query for stack: %v", err) + return structs.Space{}, fmt.Errorf("could not query for stack: %v", err) } var space structs.Space @@ -94,7 +98,20 @@ func dataCurrentSpaceRead(ctx context.Context, d *schema.ResourceData, meta inte case query.Module != nil: space = query.Module.SpaceDetails default: - return diag.Errorf("could not find stack or module with ID %s", stackID) + return structs.Space{}, fmt.Errorf("could not find stack or module with ID %s", stackID) + } + return space, nil +} + +func dataCurrentSpaceRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + stackID, err := getStackIDFromToken(meta.(*internal.Client).Token) + if err != nil { + return diag.Errorf("%v", err) + } + + space, err := getSpaceForStack(ctx, stackID, meta) + if err != nil { + return diag.Errorf("%v", err) } d.SetId(space.ID) diff --git a/spacelift/data_space_by_path.go b/spacelift/data_space_by_path.go index 572481c3..a2ecc4cf 100644 --- a/spacelift/data_space_by_path.go +++ b/spacelift/data_space_by_path.go @@ -18,6 +18,7 @@ func dataSpaceByPath() *schema.Resource { Description: "`spacelift_space_by_path` represents a Spacelift **space** - " + "a collection of resources such as stacks, modules, policies, etc. Allows for more granular access control. Can have a parent space. In contrary to `spacelift_space`, this resource is identified by a path, not by an ID. " + "For this data source to work, path must be unique. If there are multiple spaces with the same path, this datasource will fail. \n" + + "This data source can be used either with absolute paths (starting with root) or relative paths. When using a relative path, the path is relative to the current run's space. \n" + "**Disclaimer:** \n" + "This datasource can only be used in a stack that resides in a space with inheritance enabled. In addition, the parent spaces (excluding root) must also have inheritance enabled.", @@ -62,8 +63,9 @@ func dataSpaceByPath() *schema.Resource { func dataSpaceByPathRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { path := d.Get("space_path").(string) - if !strings.HasPrefix(path, "root/") && path != "root" { - return diag.Errorf("space path must start with `root`") + + if strings.HasPrefix(path, "/") { + return diag.Errorf("path must not start with a slash") } var query struct { @@ -74,7 +76,25 @@ func dataSpaceByPathRead(ctx context.Context, d *schema.ResourceData, meta inter return diag.Errorf("could not query for spaces: %v", err) } - space, err := findSpaceByPath(query.Spaces, path) + startingSpace := "root" + if !strings.HasPrefix(path, "root/") && path != "root" { + // if path does not start with root, we think it's a relative path. In this case it's relative to the current space the spacelift run is in + + stackID, err := getStackIDFromToken(meta.(*internal.Client).Token) + if err != nil { + return diag.Errorf("couldn't identify the run: %v", err) + } + + space, err := getSpaceForStack(ctx, stackID, meta) + if err != nil { + return diag.Errorf("couldn't determine current space: %v", err) + } + + startingSpace = space.ID + path = space.Name + "/" + path // to be consistent with full path search where root is always included in the path + } + + space, err := findSpaceByPath(query.Spaces, path, startingSpace) if err != nil { return diag.Errorf("error while traversing space path: %v", err) } @@ -97,12 +117,12 @@ func dataSpaceByPathRead(ctx context.Context, d *schema.ResourceData, meta inter return nil } -func findSpaceByPath(spaces []*structs.Space, path string) (*structs.Space, error) { +func findSpaceByPath(spaces []*structs.Space, path, startingSpace string) (*structs.Space, error) { childrenMap := make(map[string][]*structs.Space, len(spaces)) var currentSpace *structs.Space for _, space := range spaces { - if space.ID == "root" { + if space.ID == startingSpace { currentSpace = space } if space.ParentSpace != nil { @@ -111,7 +131,7 @@ func findSpaceByPath(spaces []*structs.Space, path string) (*structs.Space, erro } if currentSpace == nil { - return nil, fmt.Errorf("root space not found") + return nil, fmt.Errorf("%v space not found", startingSpace) } pathSplit := strings.Split(path, "/") @@ -131,7 +151,7 @@ func findSpaceByPath(spaces []*structs.Space, path string) (*structs.Space, erro } } if !found { - return nil, fmt.Errorf("could not find a space identified by path: %s", strings.Join(pathSplit[:i+1], "/")) + return nil, fmt.Errorf("space does not exist") } } diff --git a/spacelift/data_space_by_path_test.go b/spacelift/data_space_by_path_test.go index d4525c25..1520a0e7 100644 --- a/spacelift/data_space_by_path_test.go +++ b/spacelift/data_space_by_path_test.go @@ -50,7 +50,7 @@ func TestSpaceByPathData(t *testing.T) { space_path = "root123" } `, - ExpectError: regexp.MustCompile("space path must start with `root`"), + ExpectError: regexp.MustCompile("couldn't identify the run: unexpected token issuer api-key, is this a Spacelift run?"), }, { Config: ` @@ -58,7 +58,15 @@ func TestSpaceByPathData(t *testing.T) { space_path = "test123/test" } `, - ExpectError: regexp.MustCompile("space path must start with `root`"), + ExpectError: regexp.MustCompile("couldn't identify the run: unexpected token issuer api-key, is this a Spacelift run?"), + }, + { + Config: ` + data "spacelift_space_by_path" "test" { + space_path = "/my-space" + } + `, + ExpectError: regexp.MustCompile("path must not start with a slash"), }, }) }) @@ -66,8 +74,9 @@ func TestSpaceByPathData(t *testing.T) { func Test_findSpaceByPath(t *testing.T) { type args struct { - spaces []*structs.Space - path string + spaces []*structs.Space + path string + startingSpace string } var root = &structs.Space{ @@ -95,6 +104,11 @@ func Test_findSpaceByPath(t *testing.T) { Name: "rootChild", ParentSpace: &root.ID, } + var rootGrandchildAmbiguous = &structs.Space{ + ID: "rootGrandchild-randomsuffix5", + Name: "rootGrandchild", + ParentSpace: &rootChild.ID, + } tests := []struct { name string @@ -108,7 +122,8 @@ func Test_findSpaceByPath(t *testing.T) { spaces: []*structs.Space{ root, }, - path: "root", + startingSpace: "root", + path: "root", }, want: root, wantErr: false, @@ -121,7 +136,8 @@ func Test_findSpaceByPath(t *testing.T) { rootChild, rootChild2, }, - path: "root/rootChild", + startingSpace: "root", + path: "root/rootChild", }, want: rootChild, wantErr: false, @@ -135,7 +151,8 @@ func Test_findSpaceByPath(t *testing.T) { rootChild2, rootChildSameName, }, - path: "root/rootChild", + startingSpace: "root", + path: "root/rootChild", }, want: nil, wantErr: true, @@ -149,7 +166,8 @@ func Test_findSpaceByPath(t *testing.T) { rootChild2, rootGrandchild, }, - path: "root/rootChild/rootGrandchild", + startingSpace: "root", + path: "root/rootChild/rootGrandchild", }, want: rootGrandchild, wantErr: false, @@ -163,15 +181,45 @@ func Test_findSpaceByPath(t *testing.T) { rootChild2, rootGrandchild, }, - path: "root/rootGrandchild", + startingSpace: "root", + path: "root/rootGrandchild", }, want: nil, wantErr: true, }, + { + name: "grandchild should be found if starting from child", + args: args{ + spaces: []*structs.Space{ + root, + rootChild, + rootChild2, + rootGrandchild, + }, + startingSpace: rootChild.ID, + path: "rootChild/rootGrandchild", + }, + want: rootGrandchild, + wantErr: false, + }, + { + name: "ambiguous path should return error", + args: args{ + spaces: []*structs.Space{ + root, + rootChild, + rootGrandchild, + rootGrandchildAmbiguous, + }, + startingSpace: rootChild.ID, + path: "rootChild/rootGrandchild", + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := findSpaceByPath(tt.args.spaces, tt.args.path) + got, err := findSpaceByPath(tt.args.spaces, tt.args.path, tt.args.startingSpace) if (err != nil) != tt.wantErr { t.Errorf("findSpaceByPath() error = %v, wantErr %v", err, tt.wantErr) return