-
Notifications
You must be signed in to change notification settings - Fork 13
Standardize Go templating variable casing to lowercase #739
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,3 +48,54 @@ func (j *JobWithRelease) ToTemplatable() (*TemplatableJob, | |
| Release: release, | ||
| }, nil | ||
| } | ||
|
|
||
| // Map converts the TemplatableJob to a map with lowercase keys for template use. | ||
| // This provides a consistent template interface using lowercase field names | ||
| // (e.g., {{.resource.name}} instead of {{.Resource.Name}}). | ||
| func (t *TemplatableJob) Map() map[string]any { | ||
| result := make(map[string]any) | ||
|
|
||
| // Convert each field to a map using JSON marshal/unmarshal | ||
| // This ensures all keys are lowercase per JSON tags | ||
|
|
||
| // Resource | ||
| if t.Resource != nil { | ||
| result["resource"] = structToMap(t.Resource) | ||
| } | ||
|
|
||
| // Deployment | ||
| if t.Deployment != nil { | ||
| result["deployment"] = structToMap(t.Deployment) | ||
| } | ||
|
|
||
| // Environment | ||
| if t.Environment != nil { | ||
| result["environment"] = structToMap(t.Environment) | ||
| } | ||
|
|
||
| // Job | ||
| result["job"] = structToMap(t.Job) | ||
|
|
||
| // Release with variables | ||
| if t.Release != nil { | ||
| releaseMap := structToMap(t.Release.Release) | ||
| releaseMap["variables"] = t.Release.Variables | ||
| result["release"] = releaseMap | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nil map assignment causes panic in Map()Medium Severity The |
||
|
|
||
| return result | ||
| } | ||
|
|
||
| // structToMap converts a struct to a map using JSON marshal/unmarshal. | ||
| // This ensures all keys use the lowercase JSON tag names. | ||
| func structToMap(v any) map[string]any { | ||
| data, err := json.Marshal(v) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
| var result map[string]any | ||
| if err := json.Unmarshal(data, &result); err != nil { | ||
| return nil | ||
| } | ||
| return result | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| package oapi | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestTemplatableJob_Map(t *testing.T) { | ||
| now := time.Now() | ||
| job := &TemplatableJob{ | ||
| JobWithRelease: JobWithRelease{ | ||
| Job: Job{ | ||
| Id: "job-123", | ||
| ReleaseId: "release-456", | ||
| Status: JobStatusPending, | ||
| CreatedAt: now, | ||
| UpdatedAt: now, | ||
| }, | ||
| Resource: &Resource{ | ||
| Id: "resource-def", | ||
| Name: "my-app", | ||
| Identifier: "my-app-identifier", | ||
| Kind: "Kubernetes", | ||
| Version: "1.0.0", | ||
| Config: map[string]interface{}{ | ||
| "namespace": "production", | ||
| "cluster": "us-west-2", | ||
| }, | ||
| Metadata: map[string]string{ | ||
| "team": "platform", | ||
| }, | ||
| }, | ||
| Environment: &Environment{ | ||
| Id: "env-abc", | ||
| Name: "production", | ||
| }, | ||
| Deployment: &Deployment{ | ||
| Id: "deployment-789", | ||
| Name: "my-deployment", | ||
| }, | ||
| }, | ||
| Release: &TemplatableRelease{ | ||
| Release: Release{ | ||
| CreatedAt: now.Format(time.RFC3339), | ||
| ReleaseTarget: ReleaseTarget{ | ||
| DeploymentId: "deployment-789", | ||
| EnvironmentId: "env-abc", | ||
| ResourceId: "resource-def", | ||
| }, | ||
| Version: DeploymentVersion{ | ||
| Id: "version-001", | ||
| Name: "v1.2.3", | ||
| Tag: "v1.2.3", | ||
| }, | ||
| }, | ||
| Variables: map[string]string{ | ||
| "IMAGE_TAG": "v1.2.3", | ||
| "REPLICAS": "3", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| m := job.Map() | ||
| require.NotNil(t, m) | ||
|
|
||
| // Test lowercase resource fields | ||
| resource, ok := m["resource"].(map[string]any) | ||
| require.True(t, ok, "resource should be a map") | ||
| assert.Equal(t, "my-app", resource["name"]) | ||
| assert.Equal(t, "my-app-identifier", resource["identifier"]) | ||
| assert.Equal(t, "Kubernetes", resource["kind"]) | ||
|
|
||
| // Test resource config | ||
| config, ok := resource["config"].(map[string]any) | ||
| require.True(t, ok, "resource.config should be a map") | ||
| assert.Equal(t, "production", config["namespace"]) | ||
| assert.Equal(t, "us-west-2", config["cluster"]) | ||
|
|
||
| // Test lowercase environment fields | ||
| environment, ok := m["environment"].(map[string]any) | ||
| require.True(t, ok, "environment should be a map") | ||
| assert.Equal(t, "production", environment["name"]) | ||
|
|
||
| // Test lowercase deployment fields | ||
| deployment, ok := m["deployment"].(map[string]any) | ||
| require.True(t, ok, "deployment should be a map") | ||
| assert.Equal(t, "my-deployment", deployment["name"]) | ||
|
|
||
| // Test lowercase release fields | ||
| release, ok := m["release"].(map[string]any) | ||
| require.True(t, ok, "release should be a map") | ||
|
|
||
| // Test release.version | ||
| version, ok := release["version"].(map[string]any) | ||
| require.True(t, ok, "release.version should be a map") | ||
| assert.Equal(t, "v1.2.3", version["name"]) | ||
| assert.Equal(t, "v1.2.3", version["tag"]) | ||
|
|
||
| // Test release.variables | ||
| variables, ok := release["variables"].(map[string]string) | ||
| require.True(t, ok, "release.variables should be a map[string]string") | ||
| assert.Equal(t, "v1.2.3", variables["IMAGE_TAG"]) | ||
| assert.Equal(t, "3", variables["REPLICAS"]) | ||
|
|
||
| // Test lowercase job fields | ||
| jobMap, ok := m["job"].(map[string]any) | ||
| require.True(t, ok, "job should be a map") | ||
| assert.Equal(t, "job-123", jobMap["id"]) | ||
| } | ||
|
|
||
| func TestTemplatableJob_Map_NilFields(t *testing.T) { | ||
| now := time.Now() | ||
| job := &TemplatableJob{ | ||
| JobWithRelease: JobWithRelease{ | ||
| Job: Job{ | ||
| Id: "job-123", | ||
| ReleaseId: "release-456", | ||
| Status: JobStatusPending, | ||
| CreatedAt: now, | ||
| UpdatedAt: now, | ||
| }, | ||
| // Resource, Environment, Deployment are nil | ||
| }, | ||
| // Release is nil | ||
| } | ||
|
|
||
| m := job.Map() | ||
| require.NotNil(t, m) | ||
|
|
||
| // Nil fields should not be present in the map | ||
| _, ok := m["resource"] | ||
| assert.False(t, ok, "resource should not be present when nil") | ||
|
|
||
| _, ok = m["environment"] | ||
| assert.False(t, ok, "environment should not be present when nil") | ||
|
|
||
| _, ok = m["deployment"] | ||
| assert.False(t, ok, "deployment should not be present when nil") | ||
|
|
||
| _, ok = m["release"] | ||
| assert.False(t, ok, "release should not be present when nil") | ||
|
|
||
| // Job should always be present | ||
| _, ok = m["job"] | ||
| assert.True(t, ok, "job should always be present") | ||
| } | ||
|
|
||
| func TestStructToMap(t *testing.T) { | ||
| resource := &Resource{ | ||
| Id: "resource-123", | ||
| Name: "test-resource", | ||
| Identifier: "test-identifier", | ||
| Kind: "Kubernetes", | ||
| Version: "1.0.0", | ||
| } | ||
|
|
||
| m := structToMap(resource) | ||
| require.NotNil(t, m) | ||
|
|
||
| // Verify lowercase keys from JSON tags | ||
| assert.Equal(t, "resource-123", m["id"]) | ||
| assert.Equal(t, "test-resource", m["name"]) | ||
| assert.Equal(t, "test-identifier", m["identifier"]) | ||
| assert.Equal(t, "Kubernetes", m["kind"]) | ||
| assert.Equal(t, "1.0.0", m["version"]) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate the file and check its content fd job.go -p apps/workspace-engine/pkg/oapiRepository: ctrlplanedev/ctrlplane
Length of output: 103
🏁 Script executed:
Repository: ctrlplanedev/ctrlplane
Length of output: 931
🏁 Script executed:
Repository: ctrlplanedev/ctrlplane
Length of output: 253
Prevent nil-map panic when mapping release data
structToMapreturnsnilon marshal/unmarshal failure, butreleaseMap["variables"]is written unconditionally at line 82. This will panic if the JSON conversion fails. Add a nil guard before accessing the map.Proposed fix
if t.Release != nil { releaseMap := structToMap(t.Release.Release) - releaseMap["variables"] = t.Release.Variables - result["release"] = releaseMap + if releaseMap != nil { + releaseMap["variables"] = t.Release.Variables + result["release"] = releaseMap + } }📝 Committable suggestion
🤖 Prompt for AI Agents