Skip to content

chore: init variable manager template #662

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

Merged
merged 2 commits into from
Aug 22, 2025
Merged

chore: init variable manager template #662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7858f7b
chore: init variable manager template
adityachoudhari26 Aug 22, 2025
3db02ca
more
adityachoudhari26 Aug 22, 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
82 changes: 82 additions & 0 deletions apps/workspace-engine/pkg/engine/variable/deployment_variable_repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package variable

import (
"context"
"sync"
"workspace-engine/pkg/model"
"workspace-engine/pkg/model/resource"
)

type DeploymentVariable interface {
GetID() string
GetDeploymentID() string
GetKey() string
Resolve(ctx context.Context, resource *resource.Resource) (string, error)
}

var _ model.Repository[DeploymentVariable] = (*DeploymentVariableRepository)(nil)

type DeploymentVariableRepository struct {
variables map[string]*DeploymentVariable
mu sync.RWMutex
}

func NewDeploymentVariableRepository() *DeploymentVariableRepository {
return &DeploymentVariableRepository{variables: make(map[string]*DeploymentVariable)}
}

func (r *DeploymentVariableRepository) GetAll(ctx context.Context) []*DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()

return nil
}

func (r *DeploymentVariableRepository) GetAllByDeploymentID(ctx context.Context, deploymentID string) []*DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()

return nil
}

func (r *DeploymentVariableRepository) GetByDeploymentIDAndKey(ctx context.Context, deploymentID, key string) *DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()

return nil
}

func (r *DeploymentVariableRepository) Get(ctx context.Context, id string) *DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()

return nil
}

func (r *DeploymentVariableRepository) Create(ctx context.Context, variable *DeploymentVariable) error {
r.mu.Lock()
defer r.mu.Unlock()

return nil
}

func (r *DeploymentVariableRepository) Update(ctx context.Context, variable *DeploymentVariable) error {
r.mu.Lock()
defer r.mu.Unlock()

return nil
}

func (r *DeploymentVariableRepository) Delete(ctx context.Context, id string) error {
r.mu.Lock()
defer r.mu.Unlock()

return nil
}

func (r *DeploymentVariableRepository) Exists(ctx context.Context, id string) bool {
r.mu.RLock()
defer r.mu.RUnlock()

return false
}
Comment on lines +28 to +82
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 22, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Repository methods are no-ops; managers will never resolve anything

Every method currently returns zero values. As a result, getKeys returns no keys and Resolve paths will never find variables. Provide a minimal in-memory implementation to make the template usable and testable.

Apply a minimal implementation:

@@
-import (
-    "context"
-    "sync"
-    "workspace-engine/pkg/model"
-    "workspace-engine/pkg/model/resource"
-)
+import (
+    "context"
+    "fmt"
+    "sync"
+    "workspace-engine/pkg/model"
+    "workspace-engine/pkg/model/resource"
+)
@@
 func (r *DeploymentVariableRepository) GetAll(ctx context.Context) []*DeploymentVariable {
     r.mu.RLock()
     defer r.mu.RUnlock()
-
-    return nil
+    out := make([]*DeploymentVariable, 0, len(r.variables))
+    for _, v := range r.variables {
+        out = append(out, v)
+    }
+    return out
 }
@@
 func (r *DeploymentVariableRepository) GetAllByDeploymentID(ctx context.Context, deploymentID string) []*DeploymentVariable {
     r.mu.RLock()
     defer r.mu.RUnlock()
-
-    return nil
+    out := make([]*DeploymentVariable, 0)
+    for _, vptr := range r.variables {
+        if vptr == nil || *vptr == nil {
+            continue
+        }
+        v := *vptr
+        if v.GetDeploymentID() == deploymentID {
+            out = append(out, vptr)
+        }
+    }
+    return out
 }
@@
 func (r *DeploymentVariableRepository) GetByDeploymentIDAndKey(ctx context.Context, deploymentID, key string) *DeploymentVariable {
     r.mu.RLock()
     defer r.mu.RUnlock()
-
-    return nil
+    for _, vptr := range r.variables {
+        if vptr == nil || *vptr == nil {
+            continue
+        }
+        v := *vptr
+        if v.GetDeploymentID() == deploymentID && v.GetKey() == key {
+            return vptr
+        }
+    }
+    return nil
 }
@@
 func (r *DeploymentVariableRepository) Get(ctx context.Context, id string) *DeploymentVariable {
     r.mu.RLock()
     defer r.mu.RUnlock()
-
-    return nil
+    return r.variables[id]
 }
@@
 func (r *DeploymentVariableRepository) Create(ctx context.Context, variable *DeploymentVariable) error {
     r.mu.Lock()
     defer r.mu.Unlock()
-
-    return nil
+    if variable == nil || *variable == nil {
+        return fmt.Errorf("variable is nil")
+    }
+    id := (*variable).GetID()
+    if id == "" {
+        return fmt.Errorf("id cannot be empty")
+    }
+    if r.variables == nil {
+        r.variables = make(map[string]*DeploymentVariable)
+    }
+    if _, exists := r.variables[id]; exists {
+        return fmt.Errorf("deployment variable %q already exists", id)
+    }
+    r.variables[id] = variable
+    return nil
 }
@@
 func (r *DeploymentVariableRepository) Update(ctx context.Context, variable *DeploymentVariable) error {
     r.mu.Lock()
     defer r.mu.Unlock()
-
-    return nil
+    if variable == nil || *variable == nil {
+        return fmt.Errorf("variable is nil")
+    }
+    id := (*variable).GetID()
+    if id == "" {
+        return fmt.Errorf("id cannot be empty")
+    }
+    r.variables[id] = variable
+    return nil
 }
@@
 func (r *DeploymentVariableRepository) Delete(ctx context.Context, id string) error {
     r.mu.Lock()
     defer r.mu.Unlock()
-
-    return nil
+    delete(r.variables, id)
+    return nil
 }
@@
 func (r *DeploymentVariableRepository) Exists(ctx context.Context, id string) bool {
     r.mu.RLock()
     defer r.mu.RUnlock()
-
-    return false
+    _, ok := r.variables[id]
+    return ok
 }
📝 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
func (r *DeploymentVariableRepository) GetAll(ctx context.Context) []*DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()
return nil
}
func (r *DeploymentVariableRepository) GetAllByDeploymentID(ctx context.Context, deploymentID string) []*DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()
return nil
}
func (r *DeploymentVariableRepository) GetByDeploymentIDAndKey(ctx context.Context, deploymentID, key string) *DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()
return nil
}
func (r *DeploymentVariableRepository) Get(ctx context.Context, id string) *DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()
return nil
}
func (r *DeploymentVariableRepository) Create(ctx context.Context, variable *DeploymentVariable) error {
r.mu.Lock()
defer r.mu.Unlock()
return nil
}
func (r *DeploymentVariableRepository) Update(ctx context.Context, variable *DeploymentVariable) error {
r.mu.Lock()
defer r.mu.Unlock()
return nil
}
func (r *DeploymentVariableRepository) Delete(ctx context.Context, id string) error {
r.mu.Lock()
defer r.mu.Unlock()
return nil
}
func (r *DeploymentVariableRepository) Exists(ctx context.Context, id string) bool {
r.mu.RLock()
defer r.mu.RUnlock()
return false
}
import (
"context"
"fmt"
"sync"
"workspace-engine/pkg/model"
"workspace-engine/pkg/model/resource"
)
func (r *DeploymentVariableRepository) GetAll(ctx context.Context) []*DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()
out := make([]*DeploymentVariable, 0, len(r.variables))
for _, v := range r.variables {
out = append(out, v)
}
return out
}
func (r *DeploymentVariableRepository) GetAllByDeploymentID(ctx context.Context, deploymentID string) []*DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()
out := make([]*DeploymentVariable, 0)
for _, vptr := range r.variables {
if vptr == nil || *vptr == nil {
continue
}
v := *vptr
if v.GetDeploymentID() == deploymentID {
out = append(out, vptr)
}
}
return out
}
func (r *DeploymentVariableRepository) GetByDeploymentIDAndKey(ctx context.Context, deploymentID, key string) *DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()
for _, vptr := range r.variables {
if vptr == nil || *vptr == nil {
continue
}
v := *vptr
if v.GetDeploymentID() == deploymentID && v.GetKey() == key {
return vptr
}
}
return nil
}
func (r *DeploymentVariableRepository) Get(ctx context.Context, id string) *DeploymentVariable {
r.mu.RLock()
defer r.mu.RUnlock()
return r.variables[id]
}
func (r *DeploymentVariableRepository) Create(ctx context.Context, variable *DeploymentVariable) error {
r.mu.Lock()
defer r.mu.Unlock()
if variable == nil || *variable == nil {
return fmt.Errorf("variable is nil")
}
id := (*variable).GetID()
if id == "" {
return fmt.Errorf("id cannot be empty")
}
if r.variables == nil {
r.variables = make(map[string]*DeploymentVariable)
}
if _, exists := r.variables[id]; exists {
return fmt.Errorf("deployment variable %q already exists", id)
}
r.variables[id] = variable
return nil
}
func (r *DeploymentVariableRepository) Update(ctx context.Context, variable *DeploymentVariable) error {
r.mu.Lock()
defer r.mu.Unlock()
if variable == nil || *variable == nil {
return fmt.Errorf("variable is nil")
}
id := (*variable).GetID()
if id == "" {
return fmt.Errorf("id cannot be empty")
}
r.variables[id] = variable
return nil
}
func (r *DeploymentVariableRepository) Delete(ctx context.Context, id string) error {
r.mu.Lock()
defer r.mu.Unlock()
delete(r.variables, id)
return nil
}
func (r *DeploymentVariableRepository) Exists(ctx context.Context, id string) bool {
r.mu.RLock()
defer r.mu.RUnlock()
_, ok := r.variables[id]
return ok
}
🤖 Prompt for AI Agents 
In apps/workspace-engine/pkg/engine/variable/deployment_variable_repository.go
around lines 28 to 82, all repository methods are stubs returning zero values;
implement a minimal in-memory backing store using the existing r.mu (read/write
mutex) and a map field (e.g., r.store map[string]*DeploymentVariable) so
managers can resolve variables. Implement GetAll to return all values,
GetAllByDeploymentID to filter by DeploymentID, GetByDeploymentIDAndKey to find
the first matching variable by deploymentID and key, Get to return the entry by
id, Create to add a new variable (error if id already exists), Update to replace
an existing variable (error if not found), Delete to remove by id, and Exists to
check presence; use RLock for read methods and Lock for write methods and ensure
any returned slices contain the stored pointers (or copies if desired) and that
Create/Update/Delete return appropriate errors.

35 changes: 35 additions & 0 deletions apps/workspace-engine/pkg/engine/variable/manager_deployment_variable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package variable

import (
"context"
"fmt"
"workspace-engine/pkg/model/resource"
)

var _ VariableManager = (*DeploymentVariableManager)(nil)

type DeploymentVariableManager struct {
deploymentVariables *DeploymentVariableRepository
}

func NewDeploymentVariableManager(deploymentVariables *DeploymentVariableRepository) *DeploymentVariableManager {
return &DeploymentVariableManager{
deploymentVariables: deploymentVariables,
}
}

func (m *DeploymentVariableManager) Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (*string, error) {
deploymentVariablePtr := m.deploymentVariables.GetByDeploymentIDAndKey(ctx, deploymentID, key)
if deploymentVariablePtr == nil || *deploymentVariablePtr == nil {
return nil, nil
}

deploymentVariable := *deploymentVariablePtr

resolved, err := deploymentVariable.Resolve(ctx, resource)
if err != nil {
return nil, fmt.Errorf("failed to resolve deployment variable for key: %s, deploymentID: %s, err: %w", key, deploymentID, err)
}

return &resolved, nil
}
35 changes: 35 additions & 0 deletions apps/workspace-engine/pkg/engine/variable/manager_resource_variable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package variable

import (
"context"
"fmt"
"workspace-engine/pkg/model/resource"
)

var _ VariableManager = (*ResourceVariableManager)(nil)

type ResourceVariableManager struct {
resourceVariables *ResourceVariableRepository
}

func NewResourceVariableManager(resourceVariables *ResourceVariableRepository) *ResourceVariableManager {
return &ResourceVariableManager{
resourceVariables: resourceVariables,
}
}

func (m *ResourceVariableManager) Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (*string, error) {
resourceVariablePtr := m.resourceVariables.GetByResourceIDAndKey(ctx, resource.GetID(), key)
if resourceVariablePtr == nil || *resourceVariablePtr == nil {
return nil, nil
}

resourceVariable := *resourceVariablePtr

resolved, err := resourceVariable.Resolve(ctx)
if err != nil {
return nil, fmt.Errorf("failed to resolve resource variable for key: %s, resource: %s, err: %w", key, resource.GetID(), err)
}

return &resolved, nil
}
Comment on lines +21 to +35
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 22, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Nil deref risk when resource is nil

Resolve dereferences resource twice: once to fetch resource.GetID() and again in the error message. This will panic if callers pass a nil resource.

Apply this guard and make the error path nil-safe:

 func (m *ResourceVariableManager) Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (*string, error) {
-    resourceVariablePtr := m.resourceVariables.GetByResourceIDAndKey(ctx, resource.GetID(), key)
+    if resource == nil {
+        return nil, fmt.Errorf("resource is nil while resolving key: %s", key)
+    }
+    resourceVariablePtr := m.resourceVariables.GetByResourceIDAndKey(ctx, resource.GetID(), key)
@@
-    if err != nil {
-        return nil, fmt.Errorf("failed to resolve resource variable for key: %s, resource: %s, err: %w", key, resource.GetID(), err)
-    }
+    if err != nil {
+        return nil, fmt.Errorf("failed to resolve resource variable for key: %s, resource: %s, err: %w", key, resource.GetID(), err)
+    }
📝 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
func (m *ResourceVariableManager) Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (*string, error) {
resourceVariablePtr := m.resourceVariables.GetByResourceIDAndKey(ctx, resource.GetID(), key)
if resourceVariablePtr == nil || *resourceVariablePtr == nil {
return nil, nil
}
resourceVariable := *resourceVariablePtr
resolved, err := resourceVariable.Resolve(ctx)
if err != nil {
return nil, fmt.Errorf("failed to resolve resource variable for key: %s, resource: %s, err: %w", key, resource.GetID(), err)
}
return &resolved, nil
}
func (m *ResourceVariableManager) Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (*string, error) {
if resource == nil {
return nil, fmt.Errorf("resource is nil while resolving key: %s", key)
}
resourceVariablePtr := m.resourceVariables.GetByResourceIDAndKey(ctx, resource.GetID(), key)
if resourceVariablePtr == nil || *resourceVariablePtr == nil {
return nil, nil
}
resourceVariable := *resourceVariablePtr
resolved, err := resourceVariable.Resolve(ctx)
if err != nil {
return nil, fmt.Errorf("failed to resolve resource variable for key: %s, resource: %s, err: %w", key, resource.GetID(), err)
}
return &resolved, nil
}
🤖 Prompt for AI Agents 
In apps/workspace-engine/pkg/engine/variable/manager_resource_variable.go around
lines 21-35, the code dereferences resource (resource.GetID()) before checking
for nil which can panic; add a nil guard at the top (if resource == nil) and
handle that case immediately (either return a clear error or nil result as
appropriate for callers). Also capture resourceID into a local variable only
after confirming resource != nil and use that variable in the error message; if
you choose to keep an error path when resource is nil, ensure the error message
uses a safe placeholder like "<nil>" or the deploymentID instead of
dereferencing resource.

47 changes: 47 additions & 0 deletions apps/workspace-engine/pkg/engine/variable/manager_variable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package variable

import (
"context"
"fmt"
"workspace-engine/pkg/model/resource"
)

type VariableManager interface {
Resolve(ctx context.Context, resource *resource.Resource, deploymentID string, key string) (*string, error)
}

type WorkspaceVariableManager struct {
getKeys func(ctx context.Context, deploymentID string) []string
managers []VariableManager
}

func NewWorkspaceVariableManager(
getKeys func(ctx context.Context, deploymentID string) []string,
managers []VariableManager,
) *WorkspaceVariableManager {
return &WorkspaceVariableManager{
getKeys: getKeys,
managers: managers,
}
}

func (v *WorkspaceVariableManager) ResolveDeploymentVariables(ctx context.Context, resource *resource.Resource, deploymentID string) (map[string]string, error) {
resolvedVariables := make(map[string]string)

keys := v.getKeys(ctx, deploymentID)
for _, key := range keys {
for _, manager := range v.managers {
resolvedValue, err := manager.Resolve(ctx, resource, deploymentID, key)
if err != nil {
return nil, fmt.Errorf("failed to resolve for key: %s, resource: %s, err: %w", key, resource.GetID(), err)
}
Comment on lines +35 to +37
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 22, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Nil deref risk in error formatting

The error path unconditionally calls resource.GetID(). If Resolve was invoked with a nil resource, this will panic before the error can be returned.

Apply a nil-safe resource ID for error messages:

-                return nil, fmt.Errorf("failed to resolve for key: %s, resource: %s, err: %w", key, resource.GetID(), err)
+                rid := "<nil>"
+                if resource != nil {
+                    rid = resource.GetID()
+                }
+                return nil, fmt.Errorf("failed to resolve for key: %s, resource: %s, err: %w", key, rid, err)
📝 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
if err != nil {
return nil, fmt.Errorf("failed to resolve for key: %s, resource: %s, err: %w", key, resource.GetID(), err)
}
if err != nil {
rid := "<nil>"
if resource != nil {
rid = resource.GetID()
}
return nil, fmt.Errorf("failed to resolve for key: %s, resource: %s, err: %w", key, rid, err)
}
🤖 Prompt for AI Agents 
In apps/workspace-engine/pkg/engine/variable/manager_variable.go around lines 49
to 51, the error path unconditionally calls resource.GetID(), which will panic
if resource is nil; change the error construction to be nil-safe by computing an
ID string first (e.g. id := "<nil>"; if resource != nil { id = resource.GetID()
}) and then use that id in fmt.Errorf so the error path never dereferences a nil
resource.


if resolvedValue != nil {
resolvedVariables[key] = *resolvedValue
break
}
}
}

return resolvedVariables, nil
}
Loading