From 351c22cab50b0e6aa91c3292a5736b04c0b4e2b1 Mon Sep 17 00:00:00 2001 From: TIMMAREDDY DEEKSHITHA Date: Wed, 3 Jun 2026 23:24:31 +0530 Subject: [PATCH 1/8] Add Consul variable source implementation --- internal/pkg/variable/source/consul_source.go | 193 +++++++++++++++++ .../pkg/variable/source/consul_source_test.go | 197 ++++++++++++++++++ 2 files changed, 390 insertions(+) create mode 100644 internal/pkg/variable/source/consul_source.go create mode 100644 internal/pkg/variable/source/consul_source_test.go diff --git a/internal/pkg/variable/source/consul_source.go b/internal/pkg/variable/source/consul_source.go new file mode 100644 index 00000000..241b3de1 --- /dev/null +++ b/internal/pkg/variable/source/consul_source.go @@ -0,0 +1,193 @@ +// Copyright IBM Corp. 2023, 2026 +// SPDX-License-Identifier: MPL-2.0 + +package source + +import ( + "context" + "encoding/json" + "fmt" + "strings" + + "github.com/hashicorp/consul/api" + "github.com/hashicorp/nomad-pack/sdk/pack" + "github.com/hashicorp/nomad-pack/sdk/pack/variables" + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/gocty" +) + +// ConsulSource fetches variables from Consul KV store. +// Variables are stored under a configurable prefix with the structure: +// {prefix}/{pack-id}/{variable-name} +type ConsulSource struct { + name string + priority int + client *api.Client + prefix string +} + +// NewConsulSource creates a new Consul KV variable source. +// The config parameter can be nil to use default Consul configuration +// (which reads from CONSUL_HTTP_ADDR and CONSUL_HTTP_TOKEN env vars). +func NewConsulSource(priority int, config *api.Config, prefix string) (*ConsulSource, error) { + if config == nil { + config = api.DefaultConfig() + } + + client, err := api.NewClient(config) + if err != nil { + return nil, fmt.Errorf("failed to create Consul client: %w", err) + } + + if prefix == "" { + prefix = "nomad-pack/vars" + } + + return &ConsulSource{ + name: "consul", + priority: priority, + client: client, + prefix: strings.TrimSuffix(prefix, "/"), + }, nil +} + +// Name returns the unique identifier for this source. +func (c *ConsulSource) Name() string { + return c.name +} + +// Priority returns the precedence level (higher = higher priority). +func (c *ConsulSource) Priority() int { + return c.priority +} + +// Fetch retrieves variables for the given pack from Consul KV. +// Variables are expected to be stored as JSON values that can be +// converted to cty.Value types. If a value is not valid JSON, +// it will be treated as a string. +func (c *ConsulSource) Fetch(ctx context.Context, packID pack.ID) ([]*variables.Variable, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + + // Build KV path: prefix/packID/ + path := fmt.Sprintf("%s/%s/", c.prefix, packID) + + // List all keys under this path + opts := api.QueryOptions{RequireConsistent: true} + pairs, _, err := c.client.KV().List(path, (&opts).WithContext(ctx)) + + if err != nil { + return nil, fmt.Errorf("failed to list Consul KV at %s: %w", path, err) + } + + // If no keys found, return empty slice (not an error) + if len(pairs) == 0 { + return make([]*variables.Variable, 0), nil + } + + vars := make([]*variables.Variable, 0, len(pairs)) + for _, pair := range pairs { + // Extract variable name from key (remove prefix) + varName := strings.TrimPrefix(pair.Key, path) + + // Skip if this is a directory (ends with /) + if strings.HasSuffix(varName, "/") { + continue + } + + // Convert value to cty.Value + value, err := c.convertValue(pair.Value) + if err != nil { + return nil, fmt.Errorf("failed to convert value for %s: %w", varName, err) + } + + vars = append(vars, &variables.Variable{ + Name: variables.ID(varName), + Value: value, + Type: value.Type(), + }) + } + + return vars, nil +} + +// convertValue converts a byte slice to a cty.Value. +// It first attempts to parse as JSON. If that fails, it treats the value as a string. +func (c *ConsulSource) convertValue(data []byte) (cty.Value, error) { + // Try to parse as JSON first + var v interface{} + if err := json.Unmarshal(data, &v); err != nil { + // If not valid JSON, treat as string + return cty.StringVal(string(data)), nil + } + + // Convert JSON to cty.Value + return convertJSONToCty(v) +} + +// convertJSONToCty converts a Go interface{} (from JSON) to a cty.Value. +func convertJSONToCty(v interface{}) (cty.Value, error) { + switch val := v.(type) { + case nil: + return cty.NullVal(cty.DynamicPseudoType), nil + + case string: + return cty.StringVal(val), nil + + case float64: + return cty.NumberFloatVal(val), nil + + case bool: + return cty.BoolVal(val), nil + + case []interface{}: + if len(val) == 0 { + return cty.ListValEmpty(cty.DynamicPseudoType), nil + } + + // Convert each element + elements := make([]cty.Value, len(val)) + for i, elem := range val { + elemVal, err := convertJSONToCty(elem) + if err != nil { + return cty.NilVal, fmt.Errorf("failed to convert list element %d: %w", i, err) + } + elements[i] = elemVal + } + + // Try to create a list with a unified type + return cty.ListVal(elements), nil + + case map[string]interface{}: + if len(val) == 0 { + return cty.EmptyObjectVal, nil + } + + // Convert each value + attrs := make(map[string]cty.Value) + for k, v := range val { + attrVal, err := convertJSONToCty(v) + if err != nil { + return cty.NilVal, fmt.Errorf("failed to convert object attribute %s: %w", k, err) + } + attrs[k] = attrVal + } + + return cty.ObjectVal(attrs), nil + + default: + // Fallback: try to use gocty to convert + ty, err := gocty.ImpliedType(v) + if err != nil { + return cty.NilVal, fmt.Errorf("unsupported type %T: %w", v, err) + } + + ctyVal, err := gocty.ToCtyValue(v, ty) + if err != nil { + return cty.NilVal, fmt.Errorf("failed to convert %T to cty.Value: %w", v, err) + } + + return ctyVal, nil + } +} diff --git a/internal/pkg/variable/source/consul_source_test.go b/internal/pkg/variable/source/consul_source_test.go new file mode 100644 index 00000000..c0c1efe2 --- /dev/null +++ b/internal/pkg/variable/source/consul_source_test.go @@ -0,0 +1,197 @@ +// Copyright IBM Corp. 2023, 2026 +// SPDX-License-Identifier: MPL-2.0 + +package source + +import ( + "context" + "testing" + + "github.com/hashicorp/consul/api" + "github.com/hashicorp/nomad-pack/sdk/pack" + "github.com/hashicorp/nomad-pack/sdk/pack/variables" + "github.com/shoenig/test/must" + "github.com/zclconf/go-cty/cty" +) + +// skipIfConsulUnavailable checks if Consul is available and skips the test if not. +func skipIfConsulUnavailable(t *testing.T) *api.Client { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + config := api.DefaultConfig() + client, err := api.NewClient(config) + if err != nil { + t.Skipf("Consul client creation failed: %v", err) + } + + // Verify Consul is reachable + _, err = client.Status().Leader() + if err != nil { + t.Skipf("Consul not reachable: %v", err) + } + + return client +} + +func TestConsulSource_Fetch_Success(t *testing.T) { + client := skipIfConsulUnavailable(t) + + // Create source + source, err := NewConsulSource(40, nil, "nomad-pack-test/vars") + must.NoError(t, err) + + packID := pack.ID("test-pack") + kv := client.KV() + + // Setup test data + testData := map[string]string{ + "nomad-pack-test/vars/test-pack/string_var": `"hello world"`, + "nomad-pack-test/vars/test-pack/number_var": `42`, + "nomad-pack-test/vars/test-pack/bool_var": `true`, + "nomad-pack-test/vars/test-pack/list_var": `["a", "b", "c"]`, + "nomad-pack-test/vars/test-pack/map_var": `{"key1": "value1", "key2": "value2"}`, + } + + for key, value := range testData { + _, err := kv.Put(&api.KVPair{ + Key: key, + Value: []byte(value), + }, nil) + must.NoError(t, err) + } + + // Cleanup + defer func() { + _, err := kv.DeleteTree("nomad-pack-test/vars/test-pack/", nil) + must.NoError(t, err) + }() + + // Fetch variables + vars, err := source.Fetch(t.Context(), packID) + must.NoError(t, err) + must.Len(t, 5, vars) + + // Verify each variable + varMap := make(map[string]*variables.Variable) + for _, v := range vars { + varMap[string(v.Name)] = v + } + + // Check string_var + must.True(t, varMap["string_var"].Value.Equals(cty.StringVal("hello world")).True()) + + // Check number_var + must.True(t, varMap["number_var"].Value.Equals(cty.NumberIntVal(42)).True()) + + // Check bool_var + must.True(t, varMap["bool_var"].Value.Equals(cty.BoolVal(true)).True()) + + // Check list_var + expectedList := cty.ListVal([]cty.Value{ + cty.StringVal("a"), + cty.StringVal("b"), + cty.StringVal("c"), + }) + must.True(t, varMap["list_var"].Value.Equals(expectedList).True()) + + // Check map_var + expectedMap := cty.ObjectVal(map[string]cty.Value{ + "key1": cty.StringVal("value1"), + "key2": cty.StringVal("value2"), + }) + must.True(t, varMap["map_var"].Value.Equals(expectedMap).True()) +} + +func TestConsulSource_Fetch_EmptyResult(t *testing.T) { + skipIfConsulUnavailable(t) + + source, err := NewConsulSource(40, nil, "nomad-pack-test/vars") + must.NoError(t, err) + + // Fetch from non-existent pack + vars, err := source.Fetch(t.Context(), pack.ID("non-existent-pack")) + must.NoError(t, err) + must.Len(t, 0, vars) +} + +func TestConsulSource_Fetch_ContextCancellation(t *testing.T) { + skipIfConsulUnavailable(t) + + source, err := NewConsulSource(40, nil, "nomad-pack-test/vars") + must.NoError(t, err) + + // Create cancelled context + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + // Fetch should fail with context error + _, err = source.Fetch(ctx, pack.ID("test-pack")) + must.Error(t, err) +} + +func TestConsulSource_Fetch_NonJSONValue(t *testing.T) { + client := skipIfConsulUnavailable(t) + + source, err := NewConsulSource(40, nil, "nomad-pack-test/vars") + must.NoError(t, err) + + packID := pack.ID("test-pack-plain") + kv := client.KV() + + // Put non-JSON value (plain string) + _, err = kv.Put(&api.KVPair{ + Key: "nomad-pack-test/vars/test-pack-plain/plain_text", + Value: []byte("this is not json"), + }, nil) + must.NoError(t, err) + + // Cleanup + defer func() { + _, err := kv.DeleteTree("nomad-pack-test/vars/test-pack-plain/", nil) + must.NoError(t, err) + }() + + // Fetch should succeed and treat as string + vars, err := source.Fetch(t.Context(), packID) + must.NoError(t, err) + must.Len(t, 1, vars) + must.True(t, vars[0].Value.Equals(cty.StringVal("this is not json")).True()) +} + +func TestConsulSource_WithRegistry(t *testing.T) { + client := skipIfConsulUnavailable(t) + + packID := pack.ID("test-pack-registry") + kv := client.KV() + + // Setup test data in Consul + _, err := kv.Put(&api.KVPair{ + Key: "nomad-pack-test/vars/test-pack-registry/consul_var", + Value: []byte(`"from-consul"`), + }, nil) + must.NoError(t, err) + + // Cleanup + defer func() { + _, err := kv.DeleteTree("nomad-pack-test/vars/test-pack-registry/", nil) + must.NoError(t, err) + }() + + // Create registry with Consul source + registry := NewRegistry() + + consulSource, err := NewConsulSource(40, nil, "nomad-pack-test/vars") + must.NoError(t, err) + + err = registry.Register(consulSource) + must.NoError(t, err) + + // Resolve variables + vars, err := registry.Resolve(t.Context(), packID) + must.NoError(t, err) + must.Len(t, 1, vars) + must.Eq(t, "consul_var", string(vars[0].Name)) + must.True(t, vars[0].Value.Equals(cty.StringVal("from-consul")).True()) +} From 6a81a6dada9b89f5072e66b92659d4d4359e3932 Mon Sep 17 00:00:00 2001 From: TIMMAREDDY DEEKSHITHA Date: Thu, 4 Jun 2026 20:14:40 +0530 Subject: [PATCH 2/8] feat: implement Consul KV and Vault KV variable sources --- internal/pkg/variable/source/consul_source.go | 5 - .../pkg/variable/source/consul_source_test.go | 12 - internal/pkg/variable/source/vault_source.go | 235 ++++++++++++++++++ .../pkg/variable/source/vault_source_test.go | 194 +++++++++++++++ 4 files changed, 429 insertions(+), 17 deletions(-) create mode 100644 internal/pkg/variable/source/vault_source.go create mode 100644 internal/pkg/variable/source/vault_source_test.go diff --git a/internal/pkg/variable/source/consul_source.go b/internal/pkg/variable/source/consul_source.go index 241b3de1..c78d6c44 100644 --- a/internal/pkg/variable/source/consul_source.go +++ b/internal/pkg/variable/source/consul_source.go @@ -39,10 +39,6 @@ func NewConsulSource(priority int, config *api.Config, prefix string) (*ConsulSo return nil, fmt.Errorf("failed to create Consul client: %w", err) } - if prefix == "" { - prefix = "nomad-pack/vars" - } - return &ConsulSource{ name: "consul", priority: priority, @@ -177,7 +173,6 @@ func convertJSONToCty(v interface{}) (cty.Value, error) { return cty.ObjectVal(attrs), nil default: - // Fallback: try to use gocty to convert ty, err := gocty.ImpliedType(v) if err != nil { return cty.NilVal, fmt.Errorf("unsupported type %T: %w", v, err) diff --git a/internal/pkg/variable/source/consul_source_test.go b/internal/pkg/variable/source/consul_source_test.go index c0c1efe2..771b89b3 100644 --- a/internal/pkg/variable/source/consul_source_test.go +++ b/internal/pkg/variable/source/consul_source_test.go @@ -104,18 +104,6 @@ func TestConsulSource_Fetch_Success(t *testing.T) { must.True(t, varMap["map_var"].Value.Equals(expectedMap).True()) } -func TestConsulSource_Fetch_EmptyResult(t *testing.T) { - skipIfConsulUnavailable(t) - - source, err := NewConsulSource(40, nil, "nomad-pack-test/vars") - must.NoError(t, err) - - // Fetch from non-existent pack - vars, err := source.Fetch(t.Context(), pack.ID("non-existent-pack")) - must.NoError(t, err) - must.Len(t, 0, vars) -} - func TestConsulSource_Fetch_ContextCancellation(t *testing.T) { skipIfConsulUnavailable(t) diff --git a/internal/pkg/variable/source/vault_source.go b/internal/pkg/variable/source/vault_source.go new file mode 100644 index 00000000..7deaa4e5 --- /dev/null +++ b/internal/pkg/variable/source/vault_source.go @@ -0,0 +1,235 @@ +// Copyright IBM Corp. 2023, 2026 +// SPDX-License-Identifier: MPL-2.0 + +package source + +import ( + "context" + "encoding/json" + "fmt" + "strings" + + "github.com/hashicorp/nomad-pack/sdk/pack" + "github.com/hashicorp/nomad-pack/sdk/pack/variables" + vault "github.com/hashicorp/vault/api" + "github.com/zclconf/go-cty/cty" +) + +// VaultSource fetches variables from Vault KV store. +// Supports both KV v1 and KV v2 secret engines. +// Variables are stored under a configurable path with the structure: +// {mount}/{prefix}/{pack-id}/{variable-name} +type VaultSource struct { + name string + priority int + client *vault.Client + mount string + prefix string +} + +// NewVaultSource creates a new Vault KV variable source. +// The config parameter can be nil to use default Vault configuration +// (which reads from VAULT_ADDR and VAULT_TOKEN env vars). +// The mount parameter specifies the KV mount point (e.g., "secret"). +// The prefix parameter specifies the path prefix within the mount. +func NewVaultSource(priority int, config *vault.Config, mount, prefix string) (*VaultSource, error) { + if config == nil { + config = vault.DefaultConfig() + } + + client, err := vault.NewClient(config) + if err != nil { + return nil, fmt.Errorf("failed to create Vault client: %w", err) + } + + return &VaultSource{ + name: "vault", + priority: priority, + client: client, + mount: strings.Trim(mount, "/"), + prefix: strings.Trim(prefix, "/"), + }, nil +} + +// Name returns the unique identifier for this source. +func (v *VaultSource) Name() string { + return v.name +} + +// Priority returns the precedence level (higher = higher priority). +func (v *VaultSource) Priority() int { + return v.priority +} + +// Fetch retrieves variables for the given pack from Vault KV. +// Automatically detects and handles both KV v1 and KV v2 engines. +// Variables are expected to be stored as JSON values that can be +// converted to cty.Value types. If a value is not valid JSON, +// it will be treated as a string. +func (v *VaultSource) Fetch(ctx context.Context, packID pack.ID) ([]*variables.Variable, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + + // Build the base path + basePath := v.buildPath(string(packID)) + + // Try to list secrets at this path + secrets, err := v.listSecrets(ctx, basePath) + if err != nil { + return nil, fmt.Errorf("failed to list Vault secrets at %s: %w", basePath, err) + } + + // If no secrets found, return empty slice (not an error) + if len(secrets) == 0 { + return make([]*variables.Variable, 0), nil + } + + // Fetch each secret and convert to variables + vars := make([]*variables.Variable, 0, len(secrets)) + for _, secretName := range secrets { + secretPath := basePath + "/" + secretName + + // Read the secret + data, err := v.readSecret(ctx, secretPath) + if err != nil { + return nil, fmt.Errorf("failed to read secret %s: %w", secretPath, err) + } + + // Convert to cty.Value + value, err := v.convertValue(data) + if err != nil { + return nil, fmt.Errorf("failed to convert value for %s: %w", secretName, err) + } + + vars = append(vars, &variables.Variable{ + Name: variables.ID(secretName), + Value: value, + Type: value.Type(), + }) + } + + return vars, nil +} + +// buildPath constructs the full path for a pack's variables. +func (v *VaultSource) buildPath(packID string) string { + if v.prefix == "" { + return packID + } + return v.prefix + "/" + packID +} + +// listSecrets lists all secret names at the given path. +// Handles both KV v1 and KV v2 automatically. +func (v *VaultSource) listSecrets(ctx context.Context, path string) ([]string, error) { + // Try KV v2 first (most common) + listPath := v.mount + "/metadata/" + path + secret, err := v.client.Logical().ListWithContext(ctx, listPath) + + if err == nil && secret != nil && secret.Data != nil { + // KV v2 successful + return v.extractKeys(secret.Data) + } + + // Try KV v1 + listPath = v.mount + "/" + path + secret, err = v.client.Logical().ListWithContext(ctx, listPath) + + if err != nil { + return nil, err + } + + if secret == nil || secret.Data == nil { + return []string{}, nil + } + + return v.extractKeys(secret.Data) +} + +// extractKeys extracts the list of keys from Vault's list response. +func (v *VaultSource) extractKeys(data map[string]interface{}) ([]string, error) { + keysRaw, ok := data["keys"] + if !ok { + return []string{}, nil + } + + keysSlice, ok := keysRaw.([]interface{}) + if !ok { + return nil, fmt.Errorf("unexpected keys format: %T", keysRaw) + } + + keys := make([]string, 0, len(keysSlice)) + for _, k := range keysSlice { + keyStr, ok := k.(string) + if !ok { + continue + } + // Skip directories (ending with /) + if !strings.HasSuffix(keyStr, "/") { + keys = append(keys, keyStr) + } + } + + return keys, nil +} + +// readSecret reads a secret from Vault. +// Handles both KV v1 and KV v2 automatically. +func (v *VaultSource) readSecret(ctx context.Context, path string) (interface{}, error) { + // Try KV v2 first + readPath := v.mount + "/data/" + path + secret, err := v.client.Logical().ReadWithContext(ctx, readPath) + + if err == nil && secret != nil && secret.Data != nil { + // KV v2 - data is nested under "data" key + if data, ok := secret.Data["data"]; ok { + if dataMap, ok := data.(map[string]interface{}); ok { + // If there's a "value" key, use that; otherwise use the whole map + if value, ok := dataMap["value"]; ok { + return value, nil + } + return dataMap, nil + } + } + } + + // Try KV v1 + readPath = v.mount + "/" + path + secret, err = v.client.Logical().ReadWithContext(ctx, readPath) + + if err != nil { + return nil, err + } + + if secret == nil || secret.Data == nil { + return nil, fmt.Errorf("secret not found") + } + + // KV v1 - check for "value" key first, then return whole data + if value, ok := secret.Data["value"]; ok { + return value, nil + } + + return secret.Data, nil +} + +// convertValue converts a value from Vault to a cty.Value. +// Handles strings, numbers, booleans, maps, and lists. +func (v *VaultSource) convertValue(data interface{}) (cty.Value, error) { + // If it's a string, try to parse as JSON first + if str, ok := data.(string); ok { + var jsonValue interface{} + if err := json.Unmarshal([]byte(str), &jsonValue); err == nil { + // Successfully parsed as JSON + return convertJSONToCty(jsonValue) + } + // Not JSON, treat as plain string + return cty.StringVal(str), nil + } + + // For other types, convert directly + return convertJSONToCty(data) +} + +// Made with Bob diff --git a/internal/pkg/variable/source/vault_source_test.go b/internal/pkg/variable/source/vault_source_test.go new file mode 100644 index 00000000..185832b8 --- /dev/null +++ b/internal/pkg/variable/source/vault_source_test.go @@ -0,0 +1,194 @@ +// Copyright IBM Corp. 2023, 2026 +// SPDX-License-Identifier: MPL-2.0 + +package source + +import ( + "context" + "testing" + + "github.com/hashicorp/nomad-pack/sdk/pack" + "github.com/hashicorp/nomad-pack/sdk/pack/variables" + vault "github.com/hashicorp/vault/api" + "github.com/shoenig/test/must" + "github.com/zclconf/go-cty/cty" +) + +// skipIfVaultUnavailable checks if Vault is available and skips the test if not. +func skipIfVaultUnavailable(t *testing.T) *vault.Client { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + config := vault.DefaultConfig() + client, err := vault.NewClient(config) + if err != nil { + t.Skipf("Vault client creation failed: %v", err) + } + + // Verify Vault is reachable and unsealed + health, err := client.Sys().Health() + if err != nil { + t.Skipf("Vault not reachable: %v", err) + } + + if health.Sealed { + t.Skip("Vault is sealed") + } + + return client +} + +func TestVaultSource_Fetch_Success_KVv2(t *testing.T) { + client := skipIfVaultUnavailable(t) + + // Create source for KV v2 + source, err := NewVaultSource(30, nil, "secret", "nomad-pack-test/vars") + must.NoError(t, err) + + packID := pack.ID("test-pack") + + // Setup test data in Vault KV v2 + testData := map[string]map[string]interface{}{ + "string_var": {"value": "hello world"}, + "number_var": {"value": 42.0}, + "bool_var": {"value": true}, + "list_var": {"value": []interface{}{"a", "b", "c"}}, + "map_var": {"value": map[string]interface{}{"key1": "value1", "key2": "value2"}}, + } + + for key, data := range testData { + path := "secret/data/nomad-pack-test/vars/test-pack/" + key + _, err := client.Logical().Write(path, map[string]interface{}{ + "data": data, + }) + must.NoError(t, err) + } + + // Cleanup + defer func() { + for key := range testData { + path := "secret/metadata/nomad-pack-test/vars/test-pack/" + key + _, _ = client.Logical().Delete(path) + } + }() + + // Fetch variables + vars, err := source.Fetch(context.Background(), packID) + must.NoError(t, err) + must.Len(t, 5, vars) + + // Verify each variable + varMap := make(map[string]*variables.Variable) + for _, v := range vars { + varMap[string(v.Name)] = v + } + + // Check string_var + must.True(t, varMap["string_var"].Value.Equals(cty.StringVal("hello world")).True()) + + // Check number_var + must.True(t, varMap["number_var"].Value.Equals(cty.NumberFloatVal(42.0)).True()) + + // Check bool_var + must.True(t, varMap["bool_var"].Value.Equals(cty.BoolVal(true)).True()) + + // Check list_var + expectedList := cty.ListVal([]cty.Value{ + cty.StringVal("a"), + cty.StringVal("b"), + cty.StringVal("c"), + }) + must.True(t, varMap["list_var"].Value.Equals(expectedList).True()) + + // Check map_var + expectedMap := cty.ObjectVal(map[string]cty.Value{ + "key1": cty.StringVal("value1"), + "key2": cty.StringVal("value2"), + }) + must.True(t, varMap["map_var"].Value.Equals(expectedMap).True()) +} + +func TestVaultSource_Fetch_ContextCancellation(t *testing.T) { + skipIfVaultUnavailable(t) + + source, err := NewVaultSource(30, nil, "secret", "nomad-pack-test/vars") + must.NoError(t, err) + + // Create cancelled context + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + // Fetch should fail with context error + _, err = source.Fetch(ctx, pack.ID("test-pack")) + must.Error(t, err) +} + +func TestVaultSource_Fetch_StringValue(t *testing.T) { + client := skipIfVaultUnavailable(t) + + source, err := NewVaultSource(30, nil, "secret", "nomad-pack-test/vars") + must.NoError(t, err) + + packID := pack.ID("test-pack-string") + + // Put string value in Vault KV v2 + path := "secret/data/nomad-pack-test/vars/test-pack-string/plain_text" + _, err = client.Logical().Write(path, map[string]interface{}{ + "data": map[string]interface{}{ + "value": "this is plain text", + }, + }) + must.NoError(t, err) + + // Cleanup + defer func() { + path := "secret/metadata/nomad-pack-test/vars/test-pack-string/plain_text" + _, _ = client.Logical().Delete(path) + }() + + // Fetch should succeed and treat as string + vars, err := source.Fetch(context.Background(), packID) + must.NoError(t, err) + must.Len(t, 1, vars) + must.True(t, vars[0].Value.Equals(cty.StringVal("this is plain text")).True()) +} + +func TestVaultSource_WithRegistry(t *testing.T) { + client := skipIfVaultUnavailable(t) + + packID := pack.ID("test-pack-registry") + + // Setup test data in Vault + path := "secret/data/nomad-pack-test/vars/test-pack-registry/vault_var" + _, err := client.Logical().Write(path, map[string]interface{}{ + "data": map[string]interface{}{ + "value": "from-vault", + }, + }) + must.NoError(t, err) + + // Cleanup + defer func() { + path := "secret/metadata/nomad-pack-test/vars/test-pack-registry/vault_var" + _, _ = client.Logical().Delete(path) + }() + + // Create registry with Vault source + registry := NewRegistry() + + vaultSource, err := NewVaultSource(30, nil, "secret", "nomad-pack-test/vars") + must.NoError(t, err) + + err = registry.Register(vaultSource) + must.NoError(t, err) + + // Resolve variables + vars, err := registry.Resolve(context.Background(), packID) + must.NoError(t, err) + must.Len(t, 1, vars) + must.Eq(t, "vault_var", string(vars[0].Name)) + must.True(t, vars[0].Value.Equals(cty.StringVal("from-vault")).True()) +} + +// Made with Bob From 2be095106a0c687b3ede396357936d7e48af8ee5 Mon Sep 17 00:00:00 2001 From: TIMMAREDDY DEEKSHITHA Date: Fri, 5 Jun 2026 19:39:15 +0530 Subject: [PATCH 3/8] feat: Add external variable sources --- internal/pkg/variable/source/consul_source.go | 66 ------ internal/pkg/variable/source/convert.go | 78 +++++++ internal/pkg/variable/source/nomad_source.go | 147 +++++++++++++ .../pkg/variable/source/nomad_source_test.go | 197 ++++++++++++++++++ internal/pkg/variable/source/vault_source.go | 2 - .../pkg/variable/source/vault_source_test.go | 23 +- 6 files changed, 434 insertions(+), 79 deletions(-) create mode 100644 internal/pkg/variable/source/convert.go create mode 100644 internal/pkg/variable/source/nomad_source.go create mode 100644 internal/pkg/variable/source/nomad_source_test.go diff --git a/internal/pkg/variable/source/consul_source.go b/internal/pkg/variable/source/consul_source.go index c78d6c44..f585cb78 100644 --- a/internal/pkg/variable/source/consul_source.go +++ b/internal/pkg/variable/source/consul_source.go @@ -13,7 +13,6 @@ import ( "github.com/hashicorp/nomad-pack/sdk/pack" "github.com/hashicorp/nomad-pack/sdk/pack/variables" "github.com/zclconf/go-cty/cty" - "github.com/zclconf/go-cty/cty/gocty" ) // ConsulSource fetches variables from Consul KV store. @@ -121,68 +120,3 @@ func (c *ConsulSource) convertValue(data []byte) (cty.Value, error) { // Convert JSON to cty.Value return convertJSONToCty(v) } - -// convertJSONToCty converts a Go interface{} (from JSON) to a cty.Value. -func convertJSONToCty(v interface{}) (cty.Value, error) { - switch val := v.(type) { - case nil: - return cty.NullVal(cty.DynamicPseudoType), nil - - case string: - return cty.StringVal(val), nil - - case float64: - return cty.NumberFloatVal(val), nil - - case bool: - return cty.BoolVal(val), nil - - case []interface{}: - if len(val) == 0 { - return cty.ListValEmpty(cty.DynamicPseudoType), nil - } - - // Convert each element - elements := make([]cty.Value, len(val)) - for i, elem := range val { - elemVal, err := convertJSONToCty(elem) - if err != nil { - return cty.NilVal, fmt.Errorf("failed to convert list element %d: %w", i, err) - } - elements[i] = elemVal - } - - // Try to create a list with a unified type - return cty.ListVal(elements), nil - - case map[string]interface{}: - if len(val) == 0 { - return cty.EmptyObjectVal, nil - } - - // Convert each value - attrs := make(map[string]cty.Value) - for k, v := range val { - attrVal, err := convertJSONToCty(v) - if err != nil { - return cty.NilVal, fmt.Errorf("failed to convert object attribute %s: %w", k, err) - } - attrs[k] = attrVal - } - - return cty.ObjectVal(attrs), nil - - default: - ty, err := gocty.ImpliedType(v) - if err != nil { - return cty.NilVal, fmt.Errorf("unsupported type %T: %w", v, err) - } - - ctyVal, err := gocty.ToCtyValue(v, ty) - if err != nil { - return cty.NilVal, fmt.Errorf("failed to convert %T to cty.Value: %w", v, err) - } - - return ctyVal, nil - } -} diff --git a/internal/pkg/variable/source/convert.go b/internal/pkg/variable/source/convert.go new file mode 100644 index 00000000..635bdc6f --- /dev/null +++ b/internal/pkg/variable/source/convert.go @@ -0,0 +1,78 @@ +// Copyright IBM Corp. 2023, 2026 +// SPDX-License-Identifier: MPL-2.0 + +package source + +import ( + "fmt" + + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/gocty" +) + +// convertJSONToCty converts a Go interface{} (from JSON) to a cty.Value. +// This is shared by multiple source implementations that need to convert +// JSON data to HCL types. +func convertJSONToCty(v interface{}) (cty.Value, error) { + switch val := v.(type) { + case nil: + return cty.NullVal(cty.DynamicPseudoType), nil + + case string: + return cty.StringVal(val), nil + + case float64: + return cty.NumberFloatVal(val), nil + + case bool: + return cty.BoolVal(val), nil + + case []interface{}: + if len(val) == 0 { + return cty.ListValEmpty(cty.DynamicPseudoType), nil + } + + // Convert each element + elements := make([]cty.Value, len(val)) + for i, elem := range val { + elemVal, err := convertJSONToCty(elem) + if err != nil { + return cty.NilVal, fmt.Errorf("failed to convert list element %d: %w", i, err) + } + elements[i] = elemVal + } + + // Create a list with a unified type + return cty.ListVal(elements), nil + + case map[string]interface{}: + if len(val) == 0 { + return cty.EmptyObjectVal, nil + } + + // Convert each value + attrs := make(map[string]cty.Value) + for k, v := range val { + attrVal, err := convertJSONToCty(v) + if err != nil { + return cty.NilVal, fmt.Errorf("failed to convert object attribute %s: %w", k, err) + } + attrs[k] = attrVal + } + + return cty.ObjectVal(attrs), nil + + default: + ty, err := gocty.ImpliedType(v) + if err != nil { + return cty.NilVal, fmt.Errorf("unsupported type %T: %w", v, err) + } + + ctyVal, err := gocty.ToCtyValue(v, ty) + if err != nil { + return cty.NilVal, fmt.Errorf("failed to convert %T to cty.Value: %w", v, err) + } + + return ctyVal, nil + } +} diff --git a/internal/pkg/variable/source/nomad_source.go b/internal/pkg/variable/source/nomad_source.go new file mode 100644 index 00000000..19268c29 --- /dev/null +++ b/internal/pkg/variable/source/nomad_source.go @@ -0,0 +1,147 @@ +// Copyright IBM Corp. 2023, 2026 +// SPDX-License-Identifier: MPL-2.0 + +package source + +import ( + "context" + "encoding/json" + "fmt" + + "github.com/hashicorp/nomad-pack/sdk/pack" + "github.com/hashicorp/nomad-pack/sdk/pack/variables" + "github.com/hashicorp/nomad/api" + "github.com/zclconf/go-cty/cty" +) + +// NomadSource fetches variables from Nomad Variables API. +// Variables are stored under a configurable path with the structure: +// {prefix}/{pack-id}/{variable-name} +type NomadSource struct { + name string + priority int + client *api.Client + prefix string + namespace string +} + +// NewNomadSource creates a new Nomad Variables source. +// The config parameter can be nil to use default Nomad configuration +// (which reads from NOMAD_ADDR and NOMAD_TOKEN env vars). +// The prefix parameter specifies the path prefix for pack variables. +// The namespace parameter specifies the Nomad namespace to use. +func NewNomadSource(priority int, config *api.Config, prefix, namespace string) (*NomadSource, error) { + if config == nil { + config = api.DefaultConfig() + } + + client, err := api.NewClient(config) + if err != nil { + return nil, fmt.Errorf("failed to create Nomad client: %w", err) + } + + // Default namespace to "default" if not specified + if namespace == "" { + namespace = "default" + } + + return &NomadSource{ + name: "nomad", + priority: priority, + client: client, + prefix: prefix, + namespace: namespace, + }, nil +} + +// Name returns the unique identifier for this source. +func (n *NomadSource) Name() string { + return n.name +} + +// Priority returns the precedence level (higher = higher priority). +func (n *NomadSource) Priority() int { + return n.priority +} + +// Fetch retrieves variables for the given pack from Nomad Variables. +// Variables are expected to be stored as JSON-encoded values that can be +// converted to cty.Value types. +func (n *NomadSource) Fetch(ctx context.Context, packID pack.ID) ([]*variables.Variable, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + + // Build the path prefix for this pack + pathPrefix := n.buildPath(string(packID)) + + // List all variables with this prefix using the List API + opts := &api.QueryOptions{ + Namespace: n.namespace, + Prefix: pathPrefix, + } + opts = opts.WithContext(ctx) + + varList, _, err := n.client.Variables().List(opts) + if err != nil { + return nil, fmt.Errorf("failed to list Nomad variables with prefix %s: %w", pathPrefix, err) + } + + // If no variables found, return empty slice (not an error) + if len(varList) == 0 { + return make([]*variables.Variable, 0), nil + } + + // Fetch each variable and convert to pack variables + packVars := make([]*variables.Variable, 0) + for _, varMeta := range varList { + // Read the full variable + readOpts := &api.QueryOptions{ + Namespace: n.namespace, + } + readOpts = readOpts.WithContext(ctx) + + variable, _, err := n.client.Variables().Read(varMeta.Path, readOpts) + if err != nil { + return nil, fmt.Errorf("failed to read variable %s: %w", varMeta.Path, err) + } + + // Convert each item in the variable to a pack variable + for key, value := range variable.Items { + ctyVal, err := n.convertValue(value) + if err != nil { + return nil, fmt.Errorf("failed to convert value for %s: %w", key, err) + } + + packVars = append(packVars, &variables.Variable{ + Name: variables.ID(key), + Value: ctyVal, + Type: ctyVal.Type(), + }) + } + } + + return packVars, nil +} + +// buildPath constructs the full path prefix for a pack's variables. +func (n *NomadSource) buildPath(packID string) string { + if n.prefix == "" { + return packID + } + return n.prefix + "/" + packID +} + +// convertValue converts a value from Nomad Variables to a cty.Value. +// Nomad Variables stores values as strings, so we try to parse as JSON first. +func (n *NomadSource) convertValue(data string) (cty.Value, error) { + // Try to parse as JSON + var jsonValue interface{} + if err := json.Unmarshal([]byte(data), &jsonValue); err == nil { + // Successfully parsed as JSON + return convertJSONToCty(jsonValue) + } + + // Not JSON, treat as plain string + return cty.StringVal(data), nil +} diff --git a/internal/pkg/variable/source/nomad_source_test.go b/internal/pkg/variable/source/nomad_source_test.go new file mode 100644 index 00000000..25a2b618 --- /dev/null +++ b/internal/pkg/variable/source/nomad_source_test.go @@ -0,0 +1,197 @@ +// Copyright IBM Corp. 2023, 2026 +// SPDX-License-Identifier: MPL-2.0 + +package source + +import ( + "context" + "testing" + + "github.com/hashicorp/nomad-pack/sdk/pack" + "github.com/hashicorp/nomad-pack/sdk/pack/variables" + "github.com/hashicorp/nomad/api" + "github.com/hashicorp/nomad/command/agent" + "github.com/hashicorp/nomad/testutil" + "github.com/shoenig/test/must" + "github.com/zclconf/go-cty/cty" +) + +// skipIfNomadUnavailable checks if a Nomad test server can be started +func skipIfNomadUnavailable(t *testing.T) *api.Client { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + + // Start a test Nomad server + srv := agent.NewTestAgent(t, t.Name(), nil) + t.Cleanup(func() { srv.Shutdown() }) + + // Wait for leader election + testutil.WaitForLeader(t, srv.RPC) + testutil.WaitForKeyring(t, srv.RPC, srv.Config.Region) + + // Get client from test server + client := srv.APIClient() + + return client +} + +// TestNomadSource_Fetch_Success tests fetching variables from Nomad +func TestNomadSource_Fetch_Success(t *testing.T) { + client := skipIfNomadUnavailable(t) + + // Create source (pass nil to use client's config) + source, err := NewNomadSource(20, nil, "nomad-pack", "default") + must.NoError(t, err) + + // Override the client with our test client + source.client = client + + packID := pack.ID("test-pack") + + // Setup test data in Nomad Variables + testVars := []*api.Variable{ + { + Namespace: "default", + Path: "nomad-pack/test-pack/config", + Items: map[string]string{ + "string_var": `"hello world"`, + "number_var": `42`, + "bool_var": `true`, + }, + }, + { + Namespace: "default", + Path: "nomad-pack/test-pack/secrets", + Items: map[string]string{ + "list_var": `["a", "b", "c"]`, + "map_var": `{"key1": "value1", "key2": "value2"}`, + }, + }, + } + + for _, v := range testVars { + _, _, err := client.Variables().Create(v, nil) + must.NoError(t, err) + } + + // Cleanup + defer func() { + for _, v := range testVars { + _, _ = client.Variables().Delete(v.Path, nil) + } + }() + + // Fetch variables + vars, err := source.Fetch(context.Background(), packID) + must.NoError(t, err) + must.Len(t, 5, vars) + + // Verify each variable + varMap := make(map[string]*variables.Variable) + for _, v := range vars { + varMap[string(v.Name)] = v + } + + // Check string_var + must.True(t, varMap["string_var"].Value.Equals(cty.StringVal("hello world")).True()) + + // Check number_var + must.True(t, varMap["number_var"].Value.Equals(cty.NumberIntVal(42)).True()) + + // Check bool_var + must.True(t, varMap["bool_var"].Value.Equals(cty.BoolVal(true)).True()) + + // Check list_var + expectedList := cty.ListVal([]cty.Value{ + cty.StringVal("a"), + cty.StringVal("b"), + cty.StringVal("c"), + }) + must.True(t, varMap["list_var"].Value.Equals(expectedList).True()) + + // Check map_var + expectedMap := cty.ObjectVal(map[string]cty.Value{ + "key1": cty.StringVal("value1"), + "key2": cty.StringVal("value2"), + }) + must.True(t, varMap["map_var"].Value.Equals(expectedMap).True()) +} + +// TestNomadSource_Fetch_ContextCancellation tests context cancellation +func TestNomadSource_Fetch_ContextCancellation(t *testing.T) { + client := skipIfNomadUnavailable(t) + + source, err := NewNomadSource(20, nil, "nomad-pack", "default") + must.NoError(t, err) + source.client = client + + // Create a cancelled context + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + // Fetch should return context error + _, err = source.Fetch(ctx, pack.ID("test-pack")) + must.Error(t, err) +} + +// TestNomadSource_Fetch_StringValue tests plain string values +func TestNomadSource_Fetch_StringValue(t *testing.T) { + client := skipIfNomadUnavailable(t) + + source, err := NewNomadSource(20, nil, "nomad-pack", "default") + must.NoError(t, err) + source.client = client + + packID := pack.ID("test-pack-string") + + // Create a variable with plain string value (not JSON) + testVar := &api.Variable{ + Namespace: "default", + Path: "nomad-pack/test-pack-string/config", + Items: map[string]string{ + "plain_string": "just a plain string", + }, + } + + _, _, err = client.Variables().Create(testVar, nil) + must.NoError(t, err) + + defer func() { + _, _ = client.Variables().Delete(testVar.Path, nil) + }() + + // Fetch variables + vars, err := source.Fetch(context.Background(), packID) + must.NoError(t, err) + must.Len(t, 1, vars) + + // Should be treated as string + must.Eq(t, "plain_string", string(vars[0].Name)) + must.True(t, vars[0].Value.Equals(cty.StringVal("just a plain string")).True()) +} + +// TestNomadSource_WithRegistry tests integration with Registry +func TestNomadSource_WithRegistry(t *testing.T) { + client := skipIfNomadUnavailable(t) + + // Create registry and add Nomad source + registry := NewRegistry() + + nomadSource, err := NewNomadSource(20, nil, "nomad-pack", "default") + must.NoError(t, err) + nomadSource.client = client + + err = registry.Register(nomadSource) + must.NoError(t, err) + + // Verify source is registered + must.Eq(t, "nomad", nomadSource.Name()) + must.Eq(t, 20, nomadSource.Priority()) + + // Verify we can resolve from the registered source + ctx := context.Background() + vars, err := registry.Resolve(ctx, pack.ID("nonexistent")) + must.NoError(t, err) + must.Len(t, 0, vars) // No variables for nonexistent pack +} diff --git a/internal/pkg/variable/source/vault_source.go b/internal/pkg/variable/source/vault_source.go index 7deaa4e5..cf3ce739 100644 --- a/internal/pkg/variable/source/vault_source.go +++ b/internal/pkg/variable/source/vault_source.go @@ -231,5 +231,3 @@ func (v *VaultSource) convertValue(data interface{}) (cty.Value, error) { // For other types, convert directly return convertJSONToCty(data) } - -// Made with Bob diff --git a/internal/pkg/variable/source/vault_source_test.go b/internal/pkg/variable/source/vault_source_test.go index 185832b8..5e3e3316 100644 --- a/internal/pkg/variable/source/vault_source_test.go +++ b/internal/pkg/variable/source/vault_source_test.go @@ -49,18 +49,21 @@ func TestVaultSource_Fetch_Success_KVv2(t *testing.T) { packID := pack.ID("test-pack") // Setup test data in Vault KV v2 - testData := map[string]map[string]interface{}{ - "string_var": {"value": "hello world"}, - "number_var": {"value": 42.0}, - "bool_var": {"value": true}, - "list_var": {"value": []interface{}{"a", "b", "c"}}, - "map_var": {"value": map[string]interface{}{"key1": "value1", "key2": "value2"}}, + // Store values as JSON strings so they can be properly parsed + testData := map[string]string{ + "string_var": `"hello world"`, + "number_var": `42`, + "bool_var": `true`, + "list_var": `["a", "b", "c"]`, + "map_var": `{"key1": "value1", "key2": "value2"}`, } - for key, data := range testData { + for key, jsonValue := range testData { path := "secret/data/nomad-pack-test/vars/test-pack/" + key _, err := client.Logical().Write(path, map[string]interface{}{ - "data": data, + "data": map[string]interface{}{ + "value": jsonValue, + }, }) must.NoError(t, err) } @@ -88,7 +91,7 @@ func TestVaultSource_Fetch_Success_KVv2(t *testing.T) { must.True(t, varMap["string_var"].Value.Equals(cty.StringVal("hello world")).True()) // Check number_var - must.True(t, varMap["number_var"].Value.Equals(cty.NumberFloatVal(42.0)).True()) + must.True(t, varMap["number_var"].Value.Equals(cty.NumberIntVal(42)).True()) // Check bool_var must.True(t, varMap["bool_var"].Value.Equals(cty.BoolVal(true)).True()) @@ -190,5 +193,3 @@ func TestVaultSource_WithRegistry(t *testing.T) { must.Eq(t, "vault_var", string(vars[0].Name)) must.True(t, vars[0].Value.Equals(cty.StringVal("from-vault")).True()) } - -// Made with Bob From ce33d3e4cc5de6025fc1bb59cd17c790477e5f9c Mon Sep 17 00:00:00 2001 From: TIMMAREDDY DEEKSHITHA Date: Mon, 8 Jun 2026 19:45:58 +0530 Subject: [PATCH 4/8] Fix variable source implementations --- internal/pkg/variable/source/base_source.go | 6 +++--- internal/pkg/variable/source/consul_source.go | 6 +++--- internal/pkg/variable/source/nomad_source.go | 7 ++++--- internal/pkg/variable/source/vault_source.go | 8 ++++---- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/internal/pkg/variable/source/base_source.go b/internal/pkg/variable/source/base_source.go index 332759f9..d163634e 100644 --- a/internal/pkg/variable/source/base_source.go +++ b/internal/pkg/variable/source/base_source.go @@ -38,19 +38,19 @@ func (b *BaseSource) Priority() int { } // Fetch retrieves variables for the given pack from the wrapped map. -// Returns an empty slice if the pack is not found or vars is nil. +// Returns nil if the pack is not found or vars is nil. func (b *BaseSource) Fetch(ctx context.Context, packID pack.ID) ([]*variables.Variable, error) { if err := ctx.Err(); err != nil { return nil, err } if b.vars == nil { - return make([]*variables.Variable, 0), nil + return nil, nil } packVars, exists := b.vars[packID] if !exists { - return make([]*variables.Variable, 0), nil + return nil, nil } return packVars, nil diff --git a/internal/pkg/variable/source/consul_source.go b/internal/pkg/variable/source/consul_source.go index f585cb78..25d31f82 100644 --- a/internal/pkg/variable/source/consul_source.go +++ b/internal/pkg/variable/source/consul_source.go @@ -42,7 +42,7 @@ func NewConsulSource(priority int, config *api.Config, prefix string) (*ConsulSo name: "consul", priority: priority, client: client, - prefix: strings.TrimSuffix(prefix, "/"), + prefix: strings.Trim(prefix, "/"), }, nil } @@ -76,9 +76,9 @@ func (c *ConsulSource) Fetch(ctx context.Context, packID pack.ID) ([]*variables. return nil, fmt.Errorf("failed to list Consul KV at %s: %w", path, err) } - // If no keys found, return empty slice (not an error) + // If no keys found, return nil (not an error) if len(pairs) == 0 { - return make([]*variables.Variable, 0), nil + return nil, nil } vars := make([]*variables.Variable, 0, len(pairs)) diff --git a/internal/pkg/variable/source/nomad_source.go b/internal/pkg/variable/source/nomad_source.go index 19268c29..ac0cf890 100644 --- a/internal/pkg/variable/source/nomad_source.go +++ b/internal/pkg/variable/source/nomad_source.go @@ -87,13 +87,14 @@ func (n *NomadSource) Fetch(ctx context.Context, packID pack.ID) ([]*variables.V return nil, fmt.Errorf("failed to list Nomad variables with prefix %s: %w", pathPrefix, err) } - // If no variables found, return empty slice (not an error) + // If no variables found, return nil (not an error) if len(varList) == 0 { - return make([]*variables.Variable, 0), nil + return nil, nil } // Fetch each variable and convert to pack variables - packVars := make([]*variables.Variable, 0) + // Pre-allocate with estimated capacity (at least one variable per path) + packVars := make([]*variables.Variable, 0, len(varList)) for _, varMeta := range varList { // Read the full variable readOpts := &api.QueryOptions{ diff --git a/internal/pkg/variable/source/vault_source.go b/internal/pkg/variable/source/vault_source.go index cf3ce739..78028bc3 100644 --- a/internal/pkg/variable/source/vault_source.go +++ b/internal/pkg/variable/source/vault_source.go @@ -80,9 +80,9 @@ func (v *VaultSource) Fetch(ctx context.Context, packID pack.ID) ([]*variables.V return nil, fmt.Errorf("failed to list Vault secrets at %s: %w", basePath, err) } - // If no secrets found, return empty slice (not an error) + // If no secrets found, return nil (not an error) if len(secrets) == 0 { - return make([]*variables.Variable, 0), nil + return nil, nil } // Fetch each secret and convert to variables @@ -141,7 +141,7 @@ func (v *VaultSource) listSecrets(ctx context.Context, path string) ([]string, e } if secret == nil || secret.Data == nil { - return []string{}, nil + return nil, nil } return v.extractKeys(secret.Data) @@ -151,7 +151,7 @@ func (v *VaultSource) listSecrets(ctx context.Context, path string) ([]string, e func (v *VaultSource) extractKeys(data map[string]interface{}) ([]string, error) { keysRaw, ok := data["keys"] if !ok { - return []string{}, nil + return nil, nil } keysSlice, ok := keysRaw.([]interface{}) From c47eed9a0574b56089a882ea077ace226bbbd916 Mon Sep 17 00:00:00 2001 From: TIMMAREDDY DEEKSHITHA Date: Mon, 8 Jun 2026 20:14:24 +0530 Subject: [PATCH 5/8] Update CHANGELOG for external variable sources --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cbf9beb7..4dc78c76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ IMPROVEMENTS: * cli: Fixed stale-job reconciliation so nomad-pack run no longer stops active parameterized/periodic child jobs [[GH-855](https://github.com/hashicorp/nomad-pack/pull/855)] * variable: Variables declaration now supports validation stanza [[GH-841](https://github.com/hashicorp/nomad-pack/pull/841)] * variable: Add support for `nomad_variable` blocks with automatic lifecycle management during pack deployment and destruction [[GH-409](https://github.com/hashicorp/nomad-pack/pull/853)] +* variable: Add external variable sources for Consul KV, Vault KV (v1/v2), and Nomad Variables [[GH-896](https://github.com/hashicorp/nomad-pack/pull/896)] * variable: Fixed variable file overrides (last supplied wins) [[GH-851](https://github.com/hashicorp/nomad-pack/pull/851)] BUG FIXES: From 5f73449c0958d99ceaf43622a71657e87f770f28 Mon Sep 17 00:00:00 2001 From: TIMMAREDDY DEEKSHITHA Date: Tue, 9 Jun 2026 19:26:17 +0530 Subject: [PATCH 6/8] test: Improve test cleanup and error handling --- .../pkg/variable/source/consul_source_test.go | 16 ++++++++-------- internal/pkg/variable/source/convert.go | 8 ++++---- .../pkg/variable/source/nomad_source_test.go | 8 ++++---- .../pkg/variable/source/vault_source_test.go | 16 ++++++++-------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/internal/pkg/variable/source/consul_source_test.go b/internal/pkg/variable/source/consul_source_test.go index 771b89b3..e8836e65 100644 --- a/internal/pkg/variable/source/consul_source_test.go +++ b/internal/pkg/variable/source/consul_source_test.go @@ -23,13 +23,13 @@ func skipIfConsulUnavailable(t *testing.T) *api.Client { config := api.DefaultConfig() client, err := api.NewClient(config) if err != nil { - t.Skipf("Consul client creation failed: %v", err) + t.Fatalf("Consul client creation failed: %v", err) } // Verify Consul is reachable _, err = client.Status().Leader() if err != nil { - t.Skipf("Consul not reachable: %v", err) + t.Fatalf("Consul not reachable: %v", err) } return client @@ -63,10 +63,10 @@ func TestConsulSource_Fetch_Success(t *testing.T) { } // Cleanup - defer func() { + t.Cleanup(func() { _, err := kv.DeleteTree("nomad-pack-test/vars/test-pack/", nil) must.NoError(t, err) - }() + }) // Fetch variables vars, err := source.Fetch(t.Context(), packID) @@ -136,10 +136,10 @@ func TestConsulSource_Fetch_NonJSONValue(t *testing.T) { must.NoError(t, err) // Cleanup - defer func() { + t.Cleanup(func() { _, err := kv.DeleteTree("nomad-pack-test/vars/test-pack-plain/", nil) must.NoError(t, err) - }() + }) // Fetch should succeed and treat as string vars, err := source.Fetch(t.Context(), packID) @@ -162,10 +162,10 @@ func TestConsulSource_WithRegistry(t *testing.T) { must.NoError(t, err) // Cleanup - defer func() { + t.Cleanup(func() { _, err := kv.DeleteTree("nomad-pack-test/vars/test-pack-registry/", nil) must.NoError(t, err) - }() + }) // Create registry with Consul source registry := NewRegistry() diff --git a/internal/pkg/variable/source/convert.go b/internal/pkg/variable/source/convert.go index 635bdc6f..e38ca9b6 100644 --- a/internal/pkg/variable/source/convert.go +++ b/internal/pkg/variable/source/convert.go @@ -10,10 +10,10 @@ import ( "github.com/zclconf/go-cty/cty/gocty" ) -// convertJSONToCty converts a Go interface{} (from JSON) to a cty.Value. +// convertJSONToCty converts a Go any (from JSON) to a cty.Value. // This is shared by multiple source implementations that need to convert // JSON data to HCL types. -func convertJSONToCty(v interface{}) (cty.Value, error) { +func convertJSONToCty(v any) (cty.Value, error) { switch val := v.(type) { case nil: return cty.NullVal(cty.DynamicPseudoType), nil @@ -27,7 +27,7 @@ func convertJSONToCty(v interface{}) (cty.Value, error) { case bool: return cty.BoolVal(val), nil - case []interface{}: + case []any: if len(val) == 0 { return cty.ListValEmpty(cty.DynamicPseudoType), nil } @@ -45,7 +45,7 @@ func convertJSONToCty(v interface{}) (cty.Value, error) { // Create a list with a unified type return cty.ListVal(elements), nil - case map[string]interface{}: + case map[string]any: if len(val) == 0 { return cty.EmptyObjectVal, nil } diff --git a/internal/pkg/variable/source/nomad_source_test.go b/internal/pkg/variable/source/nomad_source_test.go index 25a2b618..41a0b523 100644 --- a/internal/pkg/variable/source/nomad_source_test.go +++ b/internal/pkg/variable/source/nomad_source_test.go @@ -76,11 +76,11 @@ func TestNomadSource_Fetch_Success(t *testing.T) { } // Cleanup - defer func() { + t.Cleanup(func() { for _, v := range testVars { _, _ = client.Variables().Delete(v.Path, nil) } - }() + }) // Fetch variables vars, err := source.Fetch(context.Background(), packID) @@ -157,9 +157,9 @@ func TestNomadSource_Fetch_StringValue(t *testing.T) { _, _, err = client.Variables().Create(testVar, nil) must.NoError(t, err) - defer func() { + t.Cleanup(func() { _, _ = client.Variables().Delete(testVar.Path, nil) - }() + }) // Fetch variables vars, err := source.Fetch(context.Background(), packID) diff --git a/internal/pkg/variable/source/vault_source_test.go b/internal/pkg/variable/source/vault_source_test.go index 5e3e3316..449f1efd 100644 --- a/internal/pkg/variable/source/vault_source_test.go +++ b/internal/pkg/variable/source/vault_source_test.go @@ -23,13 +23,13 @@ func skipIfVaultUnavailable(t *testing.T) *vault.Client { config := vault.DefaultConfig() client, err := vault.NewClient(config) if err != nil { - t.Skipf("Vault client creation failed: %v", err) + t.Fatalf("Vault client creation failed: %v", err) } // Verify Vault is reachable and unsealed health, err := client.Sys().Health() if err != nil { - t.Skipf("Vault not reachable: %v", err) + t.Fatalf("Vault not reachable: %v", err) } if health.Sealed { @@ -69,12 +69,12 @@ func TestVaultSource_Fetch_Success_KVv2(t *testing.T) { } // Cleanup - defer func() { + t.Cleanup(func() { for key := range testData { path := "secret/metadata/nomad-pack-test/vars/test-pack/" + key _, _ = client.Logical().Delete(path) } - }() + }) // Fetch variables vars, err := source.Fetch(context.Background(), packID) @@ -145,10 +145,10 @@ func TestVaultSource_Fetch_StringValue(t *testing.T) { must.NoError(t, err) // Cleanup - defer func() { + t.Cleanup(func() { path := "secret/metadata/nomad-pack-test/vars/test-pack-string/plain_text" _, _ = client.Logical().Delete(path) - }() + }) // Fetch should succeed and treat as string vars, err := source.Fetch(context.Background(), packID) @@ -172,10 +172,10 @@ func TestVaultSource_WithRegistry(t *testing.T) { must.NoError(t, err) // Cleanup - defer func() { + t.Cleanup(func() { path := "secret/metadata/nomad-pack-test/vars/test-pack-registry/vault_var" _, _ = client.Logical().Delete(path) - }() + }) // Create registry with Vault source registry := NewRegistry() From 32a4c20027ea10dbbffc17cfa0d2c564a0cbebd2 Mon Sep 17 00:00:00 2001 From: TIMMAREDDY DEEKSHITHA Date: Thu, 11 Jun 2026 10:08:36 +0530 Subject: [PATCH 7/8] ci: Add Consul and Vault service containers --- .github/workflows/go-tests.yml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 1077cfb3..91a73390 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -12,6 +12,29 @@ jobs: test: name: Test runs-on: ubuntu-24.04 + services: + consul: + image: hashicorp/consul:1.19 + ports: + - 8500:8500 + options: >- + --health-cmd "consul members" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + vault: + image: hashicorp/vault:1.17 + ports: + - 8200:8200 + env: + VAULT_DEV_ROOT_TOKEN_ID: root + VAULT_DEV_LISTEN_ADDRESS: 0.0.0.0:8200 + options: >- + --health-cmd "vault status" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + --cap-add=IPC_LOCK steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -34,6 +57,10 @@ jobs: # Run tests with nice formatting. Save the original log in /tmp/gotest.log - name: Run tests + env: + CONSUL_HTTP_ADDR: localhost:8500 + VAULT_ADDR: http://localhost:8200 + VAULT_TOKEN: root run: | gotestsum -f testname --jsonfile /tmp/test-output.log -- ./... From c46f4ac38b826967ebbdc57e79cde1977461af7a Mon Sep 17 00:00:00 2001 From: TIMMAREDDY DEEKSHITHA Date: Thu, 11 Jun 2026 10:15:02 +0530 Subject: [PATCH 8/8] ci: Fix Vault health check --- .github/workflows/go-tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 91a73390..9302f8b5 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -30,10 +30,10 @@ jobs: VAULT_DEV_ROOT_TOKEN_ID: root VAULT_DEV_LISTEN_ADDRESS: 0.0.0.0:8200 options: >- - --health-cmd "vault status" + --health-cmd "wget --no-verbose --tries=1 --spider http://localhost:8200/v1/sys/health || exit 1" --health-interval 10s --health-timeout 5s - --health-retries 5 + --health-retries 10 --cap-add=IPC_LOCK steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2