[Issue #473 - Subtask 2] Implement External Variable Sources (Consul KV, Vault KV, Nomad Variables)

.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 "wget --no-verbose --tries=1 --spider http://localhost:8200/v1/sys/health || exit 1"
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 10
+          --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 -- ./...
 

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:

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

internal/pkg/variable/source/consul_source.go

@@ -0,0 +1,122 @@
+// 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"
+)
+
+// 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)
+	}
+
+	return &ConsulSource{
+		name:     "consul",
+		priority: priority,
+		client:   client,
+		prefix:   strings.Trim(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 nil (not an error)
+	if len(pairs) == 0 {
+		return nil, 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)
+}

internal/pkg/variable/source/consul_source_test.go

@@ -0,0 +1,185 @@
+// 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.Fatalf("Consul client creation failed: %v", err)
+	}
+
+	// Verify Consul is reachable
+	_, err = client.Status().Leader()
+	if err != nil {
+		t.Fatalf("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
+	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)
+	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_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
+	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)
+	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
+	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()
+
+	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())
+}