diff --git a/cmd/hclexp/hclexp.go b/cmd/hclexp/hclexp.go index 36fd019..1371cc2 100644 --- a/cmd/hclexp/hclexp.go +++ b/cmd/hclexp/hclexp.go @@ -252,7 +252,15 @@ func clusterSetFromRoles(cs *hclload.ClusterSet, clusters []manifestClusterBlock dbs = append(dbs, s.Databases...) } } - cs.Add(cl.Name, dbs) + // No member role composes this cluster in the selected env: it is + // modeled elsewhere (env-varying / external), so treat it as absent — + // proxies into it are satisfied rather than failing against an empty + // schema. + if len(dbs) == 0 { + cs.AddAbsent(cl.Name) + } else { + cs.Add(cl.Name, dbs) + } for _, alias := range cl.Aliases { cs.AddAlias(alias, cl.Name) } diff --git a/cmd/hclexp/hclexp_test.go b/cmd/hclexp/hclexp_test.go index 3ee3f13..e8e3268 100644 --- a/cmd/hclexp/hclexp_test.go +++ b/cmd/hclexp/hclexp_test.go @@ -815,6 +815,128 @@ cluster "posthog" { roles = ["data"] }`) require.Len(t, byRole["data"], 2, "exactly the two real problems, nothing more") } +// #127: a cluster whose member roles have no composition in the selected env +// should resolve @absent (proxies satisfied), not register empty-but-mapped +// (proxies falsely erroring "not declared in that cluster's schema"). +func TestValidateManifest_UncomposedClusterIsAbsent(t *testing.T) { + root := t.TempDir() + // data role composes the posthog cluster, but only for env "local". + writeLayer(t, root, "layers/data/data.hcl", ` +database "posthog" { + table "sharded_events" { + order_by = ["id"] + column "id" { type = "UInt64" } + engine "merge_tree" {} + } +}`) + // ingestion role is deployed in prod-us and proxies into the posthog cluster. + writeLayer(t, root, "layers/ingestion/ing.hcl", ` +database "posthog" { + table "events" { + engine "distributed" { + cluster_name = "posthog" + remote_database = "posthog" + remote_table = "sharded_events" + } + column "id" { type = "UInt64" } + } +}`) + manifest := writeTemp(t, "roles.hcl", ` +role "data" { + env "local" { layers = ["layers/data"] } +} +role "ingestion" { + env "prod-us" { layers = ["layers/ingestion"] } +} +cluster "posthog" { roles = ["data"] }`) + + // -env prod-us: data has no prod-us composition, so posthog is uncomposed. + results, err := validateManifest(manifest, "prod-us", root, hclload.ParseSkipSet(""), hclload.ValidateOptions{}, nil) + require.NoError(t, err) + byRole := map[string][]hclload.ValidationError{} + for _, r := range results { + byRole[r.Role] = r.Errs + } + require.Empty(t, byRole["ingestion"], + "proxy into an uncomposed cluster should be satisfied (@absent), not errored") +} + +// The same cluster, in an env where its member role IS composed, validates the +// proxy against its real schema. +func TestValidateManifest_ComposedClusterValidates(t *testing.T) { + root := t.TempDir() + writeLayer(t, root, "layers/data/data.hcl", ` +database "posthog" { + table "sharded_events" { + order_by = ["id"] + column "id" { type = "UInt64" } + engine "merge_tree" {} + } + table "events" { + engine "distributed" { + cluster_name = "posthog" + remote_database = "posthog" + remote_table = "sharded_events" + } + column "id" { type = "UInt64" } + } +}`) + manifest := writeTemp(t, "roles.hcl", ` +role "data" { + env "local" { layers = ["layers/data"] } +} +cluster "posthog" { roles = ["data"] }`) + + results, err := validateManifest(manifest, "local", root, hclload.ParseSkipSet(""), hclload.ValidateOptions{}, nil) + require.NoError(t, err) + require.Len(t, results, 1) + require.Empty(t, results[0].Errs, "composed cluster: the proxy resolves against its real schema") +} + +// #127 corollary: -cluster X=@absent must override a manifest-declared X. +func TestValidateManifest_AbsentFlagOverridesManifestCluster(t *testing.T) { + root := t.TempDir() + // posthog is composed (data deployed in prod-us) but declares only 'other', + // so a proxy to sharded_events would error unless posthog is forced @absent. + writeLayer(t, root, "layers/data/data.hcl", ` +database "posthog" { + table "other" { + order_by = ["id"] + column "id" { type = "UInt64" } + engine "merge_tree" {} + } +}`) + writeLayer(t, root, "layers/ingestion/ing.hcl", ` +database "posthog" { + table "events" { + engine "distributed" { + cluster_name = "posthog" + remote_database = "posthog" + remote_table = "sharded_events" + } + column "id" { type = "UInt64" } + } +}`) + manifest := writeTemp(t, "roles.hcl", ` +role "data" { + env "prod-us" { layers = ["layers/data"] } +} +role "ingestion" { + env "prod-us" { layers = ["layers/ingestion"] } +} +cluster "posthog" { roles = ["data"] }`) + + flags := []clusterEntry{{name: "posthog", stack: absentStack}} + results, err := validateManifest(manifest, "prod-us", root, hclload.ParseSkipSet(""), hclload.ValidateOptions{}, flags) + require.NoError(t, err) + byRole := map[string][]hclload.ValidationError{} + for _, r := range results { + byRole[r.Role] = r.Errs + } + require.Empty(t, byRole["ingestion"], + "-cluster posthog=@absent should override the manifest-declared posthog and satisfy the proxy") +} + // writeLayer writes content to root/rel, creating parent dirs. func writeLayer(t *testing.T, root, rel, content string) { t.Helper() diff --git a/internal/loader/hcl/clusterset.go b/internal/loader/hcl/clusterset.go index ed3cfd4..0cbac92 100644 --- a/internal/loader/hcl/clusterset.go +++ b/internal/loader/hcl/clusterset.go @@ -36,6 +36,10 @@ func (cs *ClusterSet) Add(name string, dbs []DatabaseSpec) { } cs.declared[name] = declaredObjects(dbs) cs.resolver[name] = NewSchemaResolver(dbs) + // A name is in exactly one category; the latest writer wins so a -cluster + // flag reliably overrides a manifest-derived mapping. + delete(cs.absent, name) + delete(cs.alias, name) } // AddAbsent declares a cluster that has no composition available in this env. @@ -46,6 +50,11 @@ func (cs *ClusterSet) AddAbsent(name string) { *cs = NewClusterSet() } cs.absent[name] = true + // Clear any prior mapping/alias so `-cluster NAME=@absent` overrides a + // manifest-declared cluster (mapped is checked before absent otherwise). + delete(cs.declared, name) + delete(cs.resolver, name) + delete(cs.alias, name) } // AddAlias records that alias resolves to base: a Distributed remote on the @@ -56,6 +65,9 @@ func (cs *ClusterSet) AddAlias(alias, base string) { *cs = NewClusterSet() } cs.alias[alias] = base + delete(cs.declared, alias) + delete(cs.resolver, alias) + delete(cs.absent, alias) } // resolveCluster follows alias links to the terminal (base) cluster name. A diff --git a/internal/loader/hcl/validate_test.go b/internal/loader/hcl/validate_test.go index 276bc28..63dcf64 100644 --- a/internal/loader/hcl/validate_test.go +++ b/internal/loader/hcl/validate_test.go @@ -550,6 +550,32 @@ func TestValidate_ProxyColumns_AbsentRemoteNoColumnCheck(t *testing.T) { "@absent remote has no schema, so no column check fires") } +// A name is in exactly one category: a later AddAbsent overrides a prior Add +// (so `-cluster NAME=@absent` overrides a manifest-declared cluster). +func TestValidate_ClusterSet_AbsentOverridesMapped(t *testing.T) { + cs := NewClusterSet() + cs.Add("aux", []DatabaseSpec{mkDBMixed("posthog", + []TableSpec{mkTable("sharded", EngineMergeTree{})}, nil)}) + cs.AddAbsent("aux") // override → absent + dbs := []DatabaseSpec{mkDBMixed("posthog", + []TableSpec{mkDistTableOn("p", "aux", "posthog", "gone")}, nil)} + assert.Empty(t, Validate(dbs, ParseSkipSet(""), cs), + "AddAbsent overrides a prior Add; the proxy is satisfied") +} + +// Conversely, a later Add overrides a prior AddAbsent (a real mapping wins over +// an earlier absent marker). +func TestValidate_ClusterSet_MappedOverridesAbsent(t *testing.T) { + cs := NewClusterSet() + cs.AddAbsent("aux") + cs.Add("aux", []DatabaseSpec{mkDBMixed("posthog", + []TableSpec{mkTable("sharded", EngineMergeTree{})}, nil)}) // override → mapped + dbs := []DatabaseSpec{mkDBMixed("posthog", + []TableSpec{mkDistTableOn("p", "aux", "posthog", "gone")}, nil)} + assert.Len(t, Validate(dbs, ParseSkipSet(""), cs), 1, + "Add overrides a prior AddAbsent; the proxy is validated and the missing remote errors") +} + // A cluster marked @absent has no local composition; references into it are // structurally unresolvable and count as satisfied. func TestValidate_DistributedRemote_AbsentCluster(t *testing.T) {