Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Init system resource #5

Merged
merged 4 commits into from
Jan 30, 2025
Merged

fix: Init system resource #5

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f6b493f
fix: Init system resource
adityachoudhari26 Jan 23, 2025
d76cf13
more fix
adityachoudhari26 Jan 29, 2025
0549737
gofmt
adityachoudhari26 Jan 29, 2025
335cb41
more fix
adityachoudhari26 Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4,968 changes: 4,304 additions & 664 deletions client/client.gen.go
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions client/openapi.generate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
oapi-codegen -config openapi.client.yaml https://raw.githubusercontent.com/ctrlplanedev/ctrlplane/refs/heads/main/openapi.v1.json
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add required shell script headers and error handling.

The script is missing essential components:

  1. Shebang line to specify the shell interpreter
  2. Required headers (based on pipeline failure)
  3. Error handling for potential failures

Apply this diff to improve the script:

+#!/bin/bash
+
+# Copyright (c) 2024 CtrlPlane
+# SPDX-License-Identifier: Apache-2.0
+
+set -euo pipefail
+
+# Verify oapi-codegen is installed
+if ! command -v oapi-codegen &> /dev/null; then
+    echo "Error: oapi-codegen is not installed"
+    exit 1
+fi
+
+# Download and verify OpenAPI spec
+SPEC_URL="https://raw.githubusercontent.com/ctrlplanedev/ctrlplane/refs/heads/main/openapi.v1.json"
+TEMP_SPEC=$(mktemp)
+
+if ! curl -sSfL "$SPEC_URL" -o "$TEMP_SPEC"; then
+    echo "Error: Failed to download OpenAPI spec"
+    rm -f "$TEMP_SPEC"
+    exit 1
+fi
+
+if ! jq empty "$TEMP_SPEC" 2>/dev/null; then
+    echo "Error: Invalid JSON in OpenAPI spec"
+    rm -f "$TEMP_SPEC"
+    exit 1
+fi
+
 oapi-codegen -config openapi.client.yaml https://raw.githubusercontent.com/ctrlplanedev/ctrlplane/refs/heads/main/openapi.v1.json
+rm -f "$TEMP_SPEC"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
oapi-codegen -config openapi.client.yaml https://raw.githubusercontent.com/ctrlplanedev/ctrlplane/refs/heads/main/openapi.v1.json
#!/bin/bash
# Copyright (c) 2024 CtrlPlane
# SPDX-License-Identifier: Apache-2.0
set -euo pipefail
# Verify oapi-codegen is installed
if ! command -v oapi-codegen &> /dev/null; then
echo "Error: oapi-codegen is not installed"
exit 1
fi
# Download and verify OpenAPI spec
SPEC_URL="https://raw.githubusercontent.com/ctrlplanedev/ctrlplane/refs/heads/main/openapi.v1.json"
TEMP_SPEC=$(mktemp)
if ! curl -sSfL "$SPEC_URL" -o "$TEMP_SPEC"; then
echo "Error: Failed to download OpenAPI spec"
rm -f "$TEMP_SPEC"
exit 1
fi
if ! jq empty "$TEMP_SPEC" 2>/dev/null; then
echo "Error: Invalid JSON in OpenAPI spec"
rm -f "$TEMP_SPEC"
exit 1
fi
oapi-codegen -config openapi.client.yaml https://raw.githubusercontent.com/ctrlplanedev/ctrlplane/refs/heads/main/openapi.v1.json
rm -f "$TEMP_SPEC"
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🪛 GitHub Actions: Tests

[warning] File is missing required headers

18 changes: 16 additions & 2 deletions examples/provider/provider.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
provider "scaffolding" {
# example configuration here
terraform {
required_providers {
ctrlplane = {
source = "ctrlplane/ctrlplane"
}
}
}

Comment on lines +1 to +7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add version constraints for required provider.

The required_providers block should specify version constraints to ensure reproducible infrastructure.

Apply this diff:

 terraform {
   required_providers {
     ctrlplane = {
       source = "ctrlplane/ctrlplane"
+      version = "~> 1.0"
     }
   }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
terraform {
required_providers {
ctrlplane = {
source = "ctrlplane/ctrlplane"
}
}
}
terraform {
required_providers {
ctrlplane = {
source = "ctrlplane/ctrlplane"
version = "~> 1.0"
}
}
}

provider "ctrlplane" {
base_url = "http://localhost:3000"
workspace = "ctrlplane"
}

resource "ctrlplane_system" "example" {
name = "tf_test_official"
slug = "tf_test_official"
}
23 changes: 6 additions & 17 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
github.com/hashicorp/terraform-plugin-go v0.24.0
github.com/hashicorp/terraform-plugin-log v0.9.0
github.com/hashicorp/terraform-plugin-testing v1.10.0
github.com/hashicorp/terraform-provider-scaffolding-framework v0.0.0-20241022134422-40d1fa9ee6e4
github.com/oapi-codegen/runtime v1.1.1
)

require (
Expand All @@ -16,11 +16,8 @@ require (
github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect
github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect
github.com/cloudflare/circl v1.3.7 // indirect
github.com/dprotaso/go-yit v0.0.0-20220510233725-9ba8df137936 // indirect
github.com/fatih/color v1.16.0 // indirect
github.com/getkin/kin-openapi v0.127.0 // indirect
github.com/go-openapi/jsonpointer v0.21.0 // indirect
github.com/go-openapi/swag v0.23.0 // indirect
github.com/go-test/deep v1.0.8 // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/uuid v1.6.0 // indirect
Expand All @@ -43,27 +40,20 @@ require (
github.com/hashicorp/terraform-registry-address v0.2.3 // indirect
github.com/hashicorp/terraform-svchost v0.1.1 // indirect
github.com/hashicorp/yamux v0.1.1 // indirect
github.com/invopop/yaml v0.3.1 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/kr/text v0.2.0 // indirect
github.com/mailru/easyjson v0.7.7 // indirect
github.com/kr/pretty v0.3.1 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-isatty v0.0.20 // indirect
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/mitchellh/go-testing-interface v1.14.1 // indirect
github.com/mitchellh/go-wordwrap v1.0.0 // indirect
github.com/mitchellh/mapstructure v1.5.0 // indirect
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 // indirect
github.com/oapi-codegen/oapi-codegen/v2 v2.4.1 // indirect
github.com/oapi-codegen/runtime v1.1.1 // indirect
github.com/oklog/run v1.0.0 // indirect
github.com/perimeterx/marshmallow v1.1.5 // indirect
github.com/speakeasy-api/openapi-overlay v0.9.0 // indirect
github.com/rogpeppe/go-internal v1.12.0 // indirect
github.com/stretchr/testify v1.9.0 // indirect
github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect
github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect
github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
github.com/vmware-labs/yaml-jsonpath v0.3.2 // indirect
github.com/zclconf/go-cty v1.15.0 // indirect
golang.org/x/crypto v0.26.0 // indirect
golang.org/x/mod v0.19.0 // indirect
Expand All @@ -76,6 +66,5 @@ require (
google.golang.org/genproto/googleapis/rpc v0.0.0-20240604185151-ef581f913117 // indirect
google.golang.org/grpc v1.66.2 // indirect
google.golang.org/protobuf v1.34.2 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
)
121 changes: 10 additions & 111 deletions go.sum
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions internal/provider/data_source_model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package provider
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add the required file headers.

The pipeline indicates this file is missing required file headers. Please add the necessary headers at the top of the file to comply with project standards and pass the checks.

🧰 Tools
🪛 GitHub Actions: Tests

[warning] File is missing required headers


import (
"terraform-provider-ctrlplane/client"

"github.com/google/uuid"
)

type DataSourceModel struct {
Workspace uuid.UUID `tfsdk:"workspace"`
Client *client.ClientWithResponses `tfsdk:"client"`
}
82 changes: 76 additions & 6 deletions internal/provider/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@ package provider

import (
"context"
"errors"
"net/http"
"os"
"strings"

"terraform-provider-ctrlplane/client"

"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-framework/datasource"
"github.com/hashicorp/terraform-plugin-framework/function"
"github.com/hashicorp/terraform-plugin-framework/provider"
Expand All @@ -34,6 +37,7 @@ type CtrlplaneProvider struct {
type CtrlplaneProviderModel struct {
BaseURL types.String `tfsdk:"base_url"`
Token types.String `tfsdk:"token"`
Workspace types.String `tfsdk:"workspace"`
}

func (p *CtrlplaneProvider) Metadata(ctx context.Context, req provider.MetadataRequest, resp *provider.MetadataResponse) {
Expand All @@ -52,6 +56,10 @@ func (p *CtrlplaneProvider) Schema(ctx context.Context, req provider.SchemaReque
MarkdownDescription: "The token to use for authentication",
Optional: true,
},
"workspace": schema.StringAttribute{
MarkdownDescription: "The workspace to use",
Optional: true,
},
},
}
}
Expand All @@ -63,6 +71,51 @@ func addAPIKey(apiKey string) client.RequestEditorFn {
}
}

func getWorkspaceById(ctx context.Context, workspaceID uuid.UUID, client *client.ClientWithResponses) (uuid.UUID, error) {
validatedWorkspace, err := client.GetWorkspaceWithResponse(ctx, workspaceID)
if err != nil {
return uuid.Nil, err
}

if validatedWorkspace.JSON200 != nil {
return validatedWorkspace.JSON200.Id, nil
}

if validatedWorkspace.JSON404 != nil {
return uuid.Nil, errors.New("workspace not found")
}

return uuid.Nil, errors.New("failed to get workspace")
}
Comment on lines +74 to +89
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add context timeout handling for API calls.

The workspace retrieval functions should handle context timeouts to ensure they don't hang indefinitely on slow API responses.

+import "time"

 func getWorkspaceById(ctx context.Context, workspaceID uuid.UUID, client *client.ClientWithResponses) (uuid.UUID, error) {
+    ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
+    defer cancel()
     validatedWorkspace, err := client.GetWorkspaceWithResponse(ctx, workspaceID)
     // ... rest of the function
 }

 func getWorkspaceBySlug(ctx context.Context, slug string, client *client.ClientWithResponses) (uuid.UUID, error) {
+    ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
+    defer cancel()
     validatedWorkspace, err := client.GetWorkspaceBySlugWithResponse(ctx, slug)
     // ... rest of the function
 }

Also applies to: 91-117


func getWorkspaceBySlug(ctx context.Context, slug string, client *client.ClientWithResponses) (uuid.UUID, error) {
validatedWorkspace, err := client.GetWorkspaceBySlugWithResponse(ctx, slug)
if err != nil {
return uuid.Nil, err
}

if validatedWorkspace.JSON200 != nil {
return getWorkspaceById(ctx, validatedWorkspace.JSON200.Id, client)
}

if validatedWorkspace.JSON404 != nil {
return uuid.Nil, errors.New("workspace not found")
}

return uuid.Nil, errors.New("failed to get workspace")
}

func getWorkspace(ctx context.Context, workspace string, client *client.ClientWithResponses) (uuid.UUID, error) {
if workspace == "" {
return uuid.Nil, errors.New("workspace is required")
}

if workspaceID, err := uuid.Parse(workspace); err == nil {
return getWorkspaceById(ctx, workspaceID, client)
}
return getWorkspaceBySlug(ctx, workspace, client)
}

func (p *CtrlplaneProvider) Configure(ctx context.Context, req provider.ConfigureRequest, resp *provider.ConfigureResponse) {
var data CtrlplaneProviderModel

Expand All @@ -72,7 +125,7 @@ func (p *CtrlplaneProvider) Configure(ctx context.Context, req provider.Configur
return
}

if data.BaseURL.IsNull() {
if data.BaseURL.IsNull() {
envBaseURL := os.Getenv("CTRLPLANE_BASE_URL")
if envBaseURL != "" {
data.BaseURL = types.StringValue(envBaseURL)
Expand All @@ -90,22 +143,39 @@ func (p *CtrlplaneProvider) Configure(ctx context.Context, req provider.Configur
data.Token = types.StringValue(envToken)
}

client, err := client.NewClient(
data.BaseURL.ValueString(),
server := data.BaseURL.ValueString()
server = strings.TrimSuffix(server, "/")
server = strings.TrimSuffix(server, "/api")
server = server + "/api"
Comment on lines +146 to +149
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve URL manipulation robustness.

The current URL manipulation logic could be made more robust by using proper URL parsing.

+import "net/url"

-    server := data.BaseURL.ValueString()
-    server = strings.TrimSuffix(server, "/")
-    server = strings.TrimSuffix(server, "/api")
-    server = server + "/api"
+    baseURL, err := url.Parse(data.BaseURL.ValueString())
+    if err != nil {
+        resp.Diagnostics.AddError("Invalid base URL", err.Error())
+        return
+    }
+    baseURL.Path = strings.TrimSuffix(baseURL.Path, "/")
+    baseURL.Path = strings.TrimSuffix(baseURL.Path, "/api")
+    baseURL.Path = baseURL.Path + "/api"
+    server := baseURL.String()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
server := data.BaseURL.ValueString()
server = strings.TrimSuffix(server, "/")
server = strings.TrimSuffix(server, "/api")
server = server + "/api"
baseURL, err := url.Parse(data.BaseURL.ValueString())
if err != nil {
resp.Diagnostics.AddError("Invalid base URL", err.Error())
return
}
baseURL.Path = strings.TrimSuffix(baseURL.Path, "/")
baseURL.Path = strings.TrimSuffix(baseURL.Path, "/api")
baseURL.Path = baseURL.Path + "/api"
server := baseURL.String()


client, err := client.NewClientWithResponses(
server,
client.WithRequestEditorFn(addAPIKey(data.Token.ValueString())),
)
if err != nil {
resp.Diagnostics.AddError("Failed to create client", err.Error())
return
}

resp.DataSourceData = client
resp.ResourceData = client
configuredWorkspace := data.Workspace.ValueString()
workspaceID, err := getWorkspace(ctx, configuredWorkspace, client)
if err != nil {
resp.Diagnostics.AddError("Failed to get workspace", err.Error())
return
}

dataSourceModel := &DataSourceModel{
Workspace: workspaceID,
Client: client,
}

resp.DataSourceData = dataSourceModel
resp.ResourceData = dataSourceModel
}

func (p *CtrlplaneProvider) Resources(ctx context.Context) []func() resource.Resource {
return []func() resource.Resource{
NewTargetResource,
NewSystemResource,
}
}

Expand Down
Loading
Loading