From 0a4daac3bf62ca9ca1bbd93a5e3e025258e52e2f Mon Sep 17 00:00:00 2001 From: Gerd Oberlechner Date: Tue, 7 Jul 2026 10:17:00 +0200 Subject: [PATCH 1/3] refactor(graph): use resource group name in DOT labels MarshalDOT now takes *Graph instead of individual field arguments, and resolves the actual resource group name from ResourceGroupMeta for node labels. Node IDs still use the internal key for stability. Extract dotID and dotLabel helpers for clearer separation of identity vs display concerns. --- pipelines/graph/graph.go | 28 ++++-- pipelines/graph/graph_test.go | 8 +- .../testdata/zz_fixture_TestForEntrypoint.dot | 98 +++++++++---------- .../testdata/zz_fixture_TestForPipeline.dot | 24 ++--- 4 files changed, 86 insertions(+), 72 deletions(-) diff --git a/pipelines/graph/graph.go b/pipelines/graph/graph.go index 7031dc81..a89683b1 100644 --- a/pipelines/graph/graph.go +++ b/pipelines/graph/graph.go @@ -542,41 +542,43 @@ const graphSuffix = `}` // MarshalDOT marshals the graph described by the list of nodes into the DOT notation used by the graphviz library. // See documentation here: https://graphviz.gitlab.io/doc/info/lang.html -func MarshalDOT(nodes []Node, serviceValidationSteps map[Identifier]types.ValidationStep) ([]byte, error) { +func MarshalDOT(g *Graph) ([]byte, error) { out := bytes.Buffer{} if n, err := out.WriteString(graphPrefix); err != nil || n != len(graphPrefix) { return nil, fmt.Errorf("failed to write graph prefix: wrote %d/%d bytes: %w", n, len(graphPrefix), err) } - for _, node := range nodes { + for _, node := range g.Nodes { serviceGroup, err := shortenServiceGroup(node.ServiceGroup) if err != nil { return nil, err } - if _, err := out.WriteString(fmt.Sprintf(" \"%s_%s_%s\" [label=\"%s/%s/%s\"];\n", serviceGroup, node.ResourceGroup, node.Step, serviceGroup, node.ResourceGroup, node.Step)); err != nil { + nodeID := dotID(serviceGroup, node.Identifier) + nodeLabel := dotLabel(serviceGroup, node.Identifier, g.ResourceGroups) + if _, err := fmt.Fprintf(&out, " \"%s\" [label=\"%s\"];\n", nodeID, nodeLabel); err != nil { return nil, err } - // n.b. we don't handle parent links, as they will be written by traversing children on the parent node for _, child := range node.Children { childServiceGroup, err := shortenServiceGroup(child.ServiceGroup) if err != nil { return nil, err } - if _, err := out.WriteString(fmt.Sprintf(" \"%s_%s_%s\" -> \"%s_%s_%s\";\n", serviceGroup, node.ResourceGroup, node.Step, childServiceGroup, child.ResourceGroup, child.Step)); err != nil { + childID := dotID(childServiceGroup, child) + if _, err := fmt.Fprintf(&out, " \"%s\" -> \"%s\";\n", nodeID, childID); err != nil { return nil, err } } } - for identifier := range serviceValidationSteps { + for identifier := range g.ServiceValidationSteps { shortServiceGroup, err := shortenServiceGroup(identifier.ServiceGroup) if err != nil { return nil, err } - if _, err := out.WriteString(fmt.Sprintf(" \"serviceValidation\" -> \"%s_%s_%s\";\n", shortServiceGroup, identifier.ResourceGroup, identifier.Step)); err != nil { + if _, err := fmt.Fprintf(&out, " \"serviceValidation\" -> \"%s\";\n", dotID(shortServiceGroup, identifier)); err != nil { return nil, err } } @@ -587,6 +589,18 @@ func MarshalDOT(nodes []Node, serviceValidationSteps map[Identifier]types.Valida return out.Bytes(), nil } +func dotID(shortSG string, id Identifier) string { + return fmt.Sprintf("%s_%s_%s", shortSG, id.ResourceGroup, id.Step) +} + +func dotLabel(shortSG string, id Identifier, resourceGroups map[string]*types.ResourceGroupMeta) string { + rgName := id.ResourceGroup + if rg, ok := resourceGroups[id.ResourceGroup]; ok { + rgName = rg.ResourceGroup + } + return fmt.Sprintf("%s/%s/%s", shortSG, rgName, id.Step) +} + func shortenServiceGroup(serviceGroup string) (string, error) { parts := strings.Split(serviceGroup, ".") if len(parts) < 5 { diff --git a/pipelines/graph/graph_test.go b/pipelines/graph/graph_test.go index 975e7302..8e3aafb8 100644 --- a/pipelines/graph/graph_test.go +++ b/pipelines/graph/graph_test.go @@ -34,13 +34,13 @@ func TestForPipeline(t *testing.T) { t.Fatalf("Failed to create graph for pipeline: %v", err) } - compareGraph(t, ctx.Nodes, ctx.ServiceValidationSteps) + compareGraph(t, ctx) } -func compareGraph(t *testing.T, nodes []Node, serviceValidationSteps map[Identifier]types.ValidationStep) { +func compareGraph(t *testing.T, g *Graph) { t.Helper() - encoded, err := MarshalDOT(nodes, serviceValidationSteps) + encoded, err := MarshalDOT(g) if err != nil { t.Fatalf("Failed to marshal graph: %v", err) } @@ -84,7 +84,7 @@ func TestForEntrypoint(t *testing.T) { t.Fatalf("Failed to create graph for entrypoint: %v", err) } - compareGraph(t, ctx.Nodes, ctx.ServiceValidationSteps) + compareGraph(t, ctx) } func TestForEntrypointDuplicateResourceGroups(t *testing.T) { diff --git a/pipelines/graph/testdata/zz_fixture_TestForEntrypoint.dot b/pipelines/graph/testdata/zz_fixture_TestForEntrypoint.dot index 7cac596f..1e9a4526 100644 --- a/pipelines/graph/testdata/zz_fixture_TestForEntrypoint.dot +++ b/pipelines/graph/testdata/zz_fixture_TestForEntrypoint.dot @@ -2,25 +2,25 @@ digraph regexp { fontname="Helvetica,Arial,sans-serif" node [fontname="Helvetica,Arial,sans-serif"] edge [fontname="Helvetica,Arial,sans-serif"] - "Grandparent_global_acrs" [label="Grandparent/global/acrs"]; + "Grandparent_global_acrs" [label="Grandparent/global-rg/acrs"]; "Grandparent_global_acrs" -> "Grandparent_global_mirror-oc-mirror-image"; - "Grandparent_global_certificates" [label="Grandparent/global/certificates"]; + "Grandparent_global_certificates" [label="Grandparent/global-rg/certificates"]; "Grandparent_global_certificates" -> "Parent_global_output"; "Grandparent_global_certificates" -> "Uncle_global_output"; - "Grandparent_global_cxChildZone" [label="Grandparent/global/cxChildZone"]; + "Grandparent_global_cxChildZone" [label="Grandparent/global-rg/cxChildZone"]; "Grandparent_global_cxChildZone" -> "Parent_global_output"; "Grandparent_global_cxChildZone" -> "Uncle_global_output"; - "Grandparent_global_decrypt-and-ingest-secrets" [label="Grandparent/global/decrypt-and-ingest-secrets"]; + "Grandparent_global_decrypt-and-ingest-secrets" [label="Grandparent/global-rg/decrypt-and-ingest-secrets"]; "Grandparent_global_decrypt-and-ingest-secrets" -> "Grandparent_global_mirror-oc-mirror-image"; - "Grandparent_global_grafana-dashboards" [label="Grandparent/global/grafana-dashboards"]; - "Grandparent_global_housekeeping" [label="Grandparent/global/housekeeping"]; + "Grandparent_global_grafana-dashboards" [label="Grandparent/global-rg/grafana-dashboards"]; + "Grandparent_global_housekeeping" [label="Grandparent/global-rg/housekeeping"]; "Grandparent_global_housekeeping" -> "Grandparent_global_grafana-dashboards"; "Grandparent_global_housekeeping" -> "Parent_global_output"; "Grandparent_global_housekeeping" -> "Uncle_global_output"; - "Grandparent_global_imagemirror" [label="Grandparent/global/imagemirror"]; + "Grandparent_global_imagemirror" [label="Grandparent/global-rg/imagemirror"]; "Grandparent_global_imagemirror" -> "Parent_global_output"; "Grandparent_global_imagemirror" -> "Uncle_global_output"; - "Grandparent_global_infra" [label="Grandparent/global/infra"]; + "Grandparent_global_infra" [label="Grandparent/global-rg/infra"]; "Grandparent_global_infra" -> "Grandparent_global_acrs"; "Grandparent_global_infra" -> "Grandparent_global_cxChildZone"; "Grandparent_global_infra" -> "Grandparent_global_decrypt-and-ingest-secrets"; @@ -28,45 +28,45 @@ digraph regexp { "Grandparent_global_infra" -> "Grandparent_global_onecert-private-kv-issuer"; "Grandparent_global_infra" -> "Grandparent_global_output"; "Grandparent_global_infra" -> "Grandparent_global_svcChildZone"; - "Grandparent_global_mirror-oc-mirror-image" [label="Grandparent/global/mirror-oc-mirror-image"]; + "Grandparent_global_mirror-oc-mirror-image" [label="Grandparent/global-rg/mirror-oc-mirror-image"]; "Grandparent_global_mirror-oc-mirror-image" -> "Grandparent_global_imagemirror"; - "Grandparent_global_onecert-private-kv-issuer" [label="Grandparent/global/onecert-private-kv-issuer"]; + "Grandparent_global_onecert-private-kv-issuer" [label="Grandparent/global-rg/onecert-private-kv-issuer"]; "Grandparent_global_onecert-private-kv-issuer" -> "Grandparent_global_certificates"; - "Grandparent_global_output" [label="Grandparent/global/output"]; + "Grandparent_global_output" [label="Grandparent/global-rg/output"]; "Grandparent_global_output" -> "Grandparent_global_decrypt-and-ingest-secrets"; "Grandparent_global_output" -> "Grandparent_global_grafana-dashboards"; "Grandparent_global_output" -> "Grandparent_global_housekeeping"; "Grandparent_global_output" -> "Grandparent_global_mirror-oc-mirror-image"; - "Grandparent_global_svcChildZone" [label="Grandparent/global/svcChildZone"]; + "Grandparent_global_svcChildZone" [label="Grandparent/global-rg/svcChildZone"]; "Grandparent_global_svcChildZone" -> "Parent_global_output"; "Grandparent_global_svcChildZone" -> "Uncle_global_output"; - "Parent_global_add-hcp-grafana-datasource" [label="Parent/global/add-hcp-grafana-datasource"]; + "Parent_global_add-hcp-grafana-datasource" [label="Parent/global-rg/add-hcp-grafana-datasource"]; "Parent_global_add-hcp-grafana-datasource" -> "Child_global_output"; "Parent_global_add-hcp-grafana-datasource" -> "Child_regional_output"; "Parent_global_add-hcp-grafana-datasource" -> "Sibling_global_output"; "Parent_global_add-hcp-grafana-datasource" -> "Sibling_regional_output"; "Parent_global_add-hcp-grafana-datasource" -> "Sibling_service_output"; - "Parent_global_add-svc-grafana-datasource" [label="Parent/global/add-svc-grafana-datasource"]; + "Parent_global_add-svc-grafana-datasource" [label="Parent/global-rg/add-svc-grafana-datasource"]; "Parent_global_add-svc-grafana-datasource" -> "Parent_global_add-hcp-grafana-datasource"; - "Parent_global_ocp-acr-replication" [label="Parent/global/ocp-acr-replication"]; + "Parent_global_ocp-acr-replication" [label="Parent/global-rg/ocp-acr-replication"]; "Parent_global_ocp-acr-replication" -> "Parent_regional_rpRegistration"; - "Parent_global_output" [label="Parent/global/output"]; + "Parent_global_output" [label="Parent/global-rg/output"]; "Parent_global_output" -> "Parent_global_add-hcp-grafana-datasource"; "Parent_global_output" -> "Parent_global_add-svc-grafana-datasource"; "Parent_global_output" -> "Parent_global_ocp-acr-replication"; "Parent_global_output" -> "Parent_global_svc-acr-replication"; "Parent_global_output" -> "Parent_regional_region"; "Parent_global_output" -> "Parent_regional_rpRegistration"; - "Parent_global_svc-acr-replication" [label="Parent/global/svc-acr-replication"]; + "Parent_global_svc-acr-replication" [label="Parent/global-rg/svc-acr-replication"]; "Parent_global_svc-acr-replication" -> "Parent_regional_rpRegistration"; - "Parent_regional_output" [label="Parent/regional/output"]; + "Parent_regional_output" [label="Parent/regional-rg/output"]; "Parent_regional_output" -> "Parent_global_add-hcp-grafana-datasource"; "Parent_regional_output" -> "Parent_global_add-svc-grafana-datasource"; - "Parent_regional_region" [label="Parent/regional/region"]; + "Parent_regional_region" [label="Parent/regional-rg/region"]; "Parent_regional_region" -> "Parent_regional_output"; - "Parent_regional_rpRegistration" [label="Parent/regional/rpRegistration"]; + "Parent_regional_rpRegistration" [label="Parent/regional-rg/rpRegistration"]; "Parent_regional_rpRegistration" -> "Parent_regional_region"; - "Child_global_output" [label="Child/global/output"]; + "Child_global_output" [label="Child/global-rg/output"]; "Child_global_output" -> "Child_service_acrpull"; "Child_global_output" -> "Child_service_arobit"; "Child_global_output" -> "Child_service_infra"; @@ -74,28 +74,28 @@ digraph regexp { "Child_global_output" -> "Child_service_istio-upgrade"; "Child_global_output" -> "Child_service_prometheus"; "Child_global_output" -> "Child_service_svc"; - "Child_regional_output" [label="Child/regional/output"]; + "Child_regional_output" [label="Child/regional-rg/output"]; "Child_regional_output" -> "Child_service_infra"; "Child_regional_output" -> "Child_service_svc"; - "Child_service_acrpull" [label="Child/service/acrpull"]; - "Child_service_arobit" [label="Child/service/arobit"]; - "Child_service_infra" [label="Child/service/infra"]; + "Child_service_acrpull" [label="Child/service-rg/acrpull"]; + "Child_service_arobit" [label="Child/service-rg/arobit"]; + "Child_service_infra" [label="Child/service-rg/infra"]; "Child_service_infra" -> "Child_service_svc-oncert-private-kv-issuer"; "Child_service_infra" -> "Child_service_svc-oncert-public-kv-issuer"; - "Child_service_istio-config" [label="Child/service/istio-config"]; + "Child_service_istio-config" [label="Child/service-rg/istio-config"]; "Child_service_istio-config" -> "Child_service_istio-upgrade"; - "Child_service_istio-upgrade" [label="Child/service/istio-upgrade"]; - "Child_service_prometheus" [label="Child/service/prometheus"]; + "Child_service_istio-upgrade" [label="Child/service-rg/istio-upgrade"]; + "Child_service_prometheus" [label="Child/service-rg/prometheus"]; "Child_service_prometheus" -> "Child_service_acrpull"; "Child_service_prometheus" -> "Child_service_istio-config"; - "Child_service_svc" [label="Child/service/svc"]; + "Child_service_svc" [label="Child/service-rg/svc"]; "Child_service_svc" -> "Child_service_arobit"; "Child_service_svc" -> "Child_service_prometheus"; - "Child_service_svc-oncert-private-kv-issuer" [label="Child/service/svc-oncert-private-kv-issuer"]; + "Child_service_svc-oncert-private-kv-issuer" [label="Child/service-rg/svc-oncert-private-kv-issuer"]; "Child_service_svc-oncert-private-kv-issuer" -> "Child_service_svc"; - "Child_service_svc-oncert-public-kv-issuer" [label="Child/service/svc-oncert-public-kv-issuer"]; + "Child_service_svc-oncert-public-kv-issuer" [label="Child/service-rg/svc-oncert-public-kv-issuer"]; "Child_service_svc-oncert-public-kv-issuer" -> "Child_service_svc"; - "Sibling_global_output" [label="Sibling/global/output"]; + "Sibling_global_output" [label="Sibling/global-rg/output"]; "Sibling_global_output" -> "Sibling_management_acrpull"; "Sibling_global_output" -> "Sibling_management_arobit"; "Sibling_global_output" -> "Sibling_management_mgmt-cluster"; @@ -103,45 +103,45 @@ digraph regexp { "Sibling_global_output" -> "Sibling_management_mgmt-infra"; "Sibling_global_output" -> "Sibling_management_prometheus"; "Sibling_global_output" -> "Sibling_management_rpRegistration"; - "Sibling_management_acrpull" [label="Sibling/management/acrpull"]; - "Sibling_management_arobit" [label="Sibling/management/arobit"]; - "Sibling_management_cx-oncert-public-kv-issuer" [label="Sibling/management/cx-oncert-public-kv-issuer"]; + "Sibling_management_acrpull" [label="Sibling/management-rg/acrpull"]; + "Sibling_management_arobit" [label="Sibling/management-rg/arobit"]; + "Sibling_management_cx-oncert-public-kv-issuer" [label="Sibling/management-rg/cx-oncert-public-kv-issuer"]; "Sibling_management_cx-oncert-public-kv-issuer" -> "Sibling_management_mgmt-cluster"; - "Sibling_management_mgmt-cluster" [label="Sibling/management/mgmt-cluster"]; + "Sibling_management_mgmt-cluster" [label="Sibling/management-rg/mgmt-cluster"]; "Sibling_management_mgmt-cluster" -> "Sibling_management_arobit"; "Sibling_management_mgmt-cluster" -> "Sibling_management_mgmt-fixes"; "Sibling_management_mgmt-cluster" -> "Sibling_management_mgmt-nsp"; "Sibling_management_mgmt-cluster" -> "Sibling_management_prometheus"; - "Sibling_management_mgmt-fixes" [label="Sibling/management/mgmt-fixes"]; - "Sibling_management_mgmt-infra" [label="Sibling/management/mgmt-infra"]; + "Sibling_management_mgmt-fixes" [label="Sibling/management-rg/mgmt-fixes"]; + "Sibling_management_mgmt-infra" [label="Sibling/management-rg/mgmt-infra"]; "Sibling_management_mgmt-infra" -> "Sibling_management_cx-oncert-public-kv-issuer"; "Sibling_management_mgmt-infra" -> "Sibling_management_mgmt-nsp"; "Sibling_management_mgmt-infra" -> "Sibling_management_mgmt-oncert-private-kv-issuer"; "Sibling_management_mgmt-infra" -> "Sibling_management_mgmt-oncert-public-kv-issuer"; - "Sibling_management_mgmt-nsp" [label="Sibling/management/mgmt-nsp"]; - "Sibling_management_mgmt-oncert-private-kv-issuer" [label="Sibling/management/mgmt-oncert-private-kv-issuer"]; + "Sibling_management_mgmt-nsp" [label="Sibling/management-rg/mgmt-nsp"]; + "Sibling_management_mgmt-oncert-private-kv-issuer" [label="Sibling/management-rg/mgmt-oncert-private-kv-issuer"]; "Sibling_management_mgmt-oncert-private-kv-issuer" -> "Sibling_management_mgmt-cluster"; - "Sibling_management_mgmt-oncert-public-kv-issuer" [label="Sibling/management/mgmt-oncert-public-kv-issuer"]; + "Sibling_management_mgmt-oncert-public-kv-issuer" [label="Sibling/management-rg/mgmt-oncert-public-kv-issuer"]; "Sibling_management_mgmt-oncert-public-kv-issuer" -> "Sibling_management_mgmt-cluster"; - "Sibling_management_prometheus" [label="Sibling/management/prometheus"]; + "Sibling_management_prometheus" [label="Sibling/management-rg/prometheus"]; "Sibling_management_prometheus" -> "Sibling_management_acrpull"; - "Sibling_management_rpRegistration" [label="Sibling/management/rpRegistration"]; + "Sibling_management_rpRegistration" [label="Sibling/management-rg/rpRegistration"]; "Sibling_management_rpRegistration" -> "Sibling_management_mgmt-infra"; - "Sibling_regional_output" [label="Sibling/regional/output"]; + "Sibling_regional_output" [label="Sibling/regional-rg/output"]; "Sibling_regional_output" -> "Sibling_management_mgmt-cluster"; "Sibling_regional_output" -> "Sibling_management_mgmt-infra"; - "Sibling_service_output" [label="Sibling/service/output"]; + "Sibling_service_output" [label="Sibling/service-rg/output"]; "Sibling_service_output" -> "Sibling_management_mgmt-infra"; "Sibling_service_output" -> "Sibling_management_mgmt-nsp"; - "Uncle_global_output" [label="Uncle/global/output"]; + "Uncle_global_output" [label="Uncle/global-rg/output"]; "Uncle_global_output" -> "Sibling_management_mgmt-nsp"; "Uncle_global_output" -> "Uncle_global_rg-ownership"; "Uncle_global_output" -> "Uncle_regional_delete-region"; "Uncle_global_output" -> "Uncle_regional_rg-ownership"; - "Uncle_global_rg-ownership" [label="Uncle/global/rg-ownership"]; + "Uncle_global_rg-ownership" [label="Uncle/global-rg/rg-ownership"]; "Uncle_global_rg-ownership" -> "Uncle_regional_delete-region"; - "Uncle_regional_delete-region" [label="Uncle/regional/delete-region"]; - "Uncle_regional_rg-ownership" [label="Uncle/regional/rg-ownership"]; + "Uncle_regional_delete-region" [label="Uncle/regional-rg/delete-region"]; + "Uncle_regional_rg-ownership" [label="Uncle/regional-rg/rg-ownership"]; "Uncle_regional_rg-ownership" -> "Uncle_regional_delete-region"; "serviceValidation" -> "Child_service_validation"; } \ No newline at end of file diff --git a/pipelines/graph/testdata/zz_fixture_TestForPipeline.dot b/pipelines/graph/testdata/zz_fixture_TestForPipeline.dot index 2349f7da..0d9f68b0 100644 --- a/pipelines/graph/testdata/zz_fixture_TestForPipeline.dot +++ b/pipelines/graph/testdata/zz_fixture_TestForPipeline.dot @@ -2,17 +2,17 @@ digraph regexp { fontname="Helvetica,Arial,sans-serif" node [fontname="Helvetica,Arial,sans-serif"] edge [fontname="Helvetica,Arial,sans-serif"] - "Grandparent_global_acrs" [label="Grandparent/global/acrs"]; + "Grandparent_global_acrs" [label="Grandparent/global-rg/acrs"]; "Grandparent_global_acrs" -> "Grandparent_global_mirror-oc-mirror-image"; - "Grandparent_global_certificates" [label="Grandparent/global/certificates"]; - "Grandparent_global_cxChildZone" [label="Grandparent/global/cxChildZone"]; - "Grandparent_global_decrypt-and-ingest-secrets" [label="Grandparent/global/decrypt-and-ingest-secrets"]; + "Grandparent_global_certificates" [label="Grandparent/global-rg/certificates"]; + "Grandparent_global_cxChildZone" [label="Grandparent/global-rg/cxChildZone"]; + "Grandparent_global_decrypt-and-ingest-secrets" [label="Grandparent/global-rg/decrypt-and-ingest-secrets"]; "Grandparent_global_decrypt-and-ingest-secrets" -> "Grandparent_global_mirror-oc-mirror-image"; - "Grandparent_global_grafana-dashboards" [label="Grandparent/global/grafana-dashboards"]; - "Grandparent_global_housekeeping" [label="Grandparent/global/housekeeping"]; + "Grandparent_global_grafana-dashboards" [label="Grandparent/global-rg/grafana-dashboards"]; + "Grandparent_global_housekeeping" [label="Grandparent/global-rg/housekeeping"]; "Grandparent_global_housekeeping" -> "Grandparent_global_grafana-dashboards"; - "Grandparent_global_imagemirror" [label="Grandparent/global/imagemirror"]; - "Grandparent_global_infra" [label="Grandparent/global/infra"]; + "Grandparent_global_imagemirror" [label="Grandparent/global-rg/imagemirror"]; + "Grandparent_global_infra" [label="Grandparent/global-rg/infra"]; "Grandparent_global_infra" -> "Grandparent_global_acrs"; "Grandparent_global_infra" -> "Grandparent_global_cxChildZone"; "Grandparent_global_infra" -> "Grandparent_global_decrypt-and-ingest-secrets"; @@ -20,14 +20,14 @@ digraph regexp { "Grandparent_global_infra" -> "Grandparent_global_onecert-private-kv-issuer"; "Grandparent_global_infra" -> "Grandparent_global_output"; "Grandparent_global_infra" -> "Grandparent_global_svcChildZone"; - "Grandparent_global_mirror-oc-mirror-image" [label="Grandparent/global/mirror-oc-mirror-image"]; + "Grandparent_global_mirror-oc-mirror-image" [label="Grandparent/global-rg/mirror-oc-mirror-image"]; "Grandparent_global_mirror-oc-mirror-image" -> "Grandparent_global_imagemirror"; - "Grandparent_global_onecert-private-kv-issuer" [label="Grandparent/global/onecert-private-kv-issuer"]; + "Grandparent_global_onecert-private-kv-issuer" [label="Grandparent/global-rg/onecert-private-kv-issuer"]; "Grandparent_global_onecert-private-kv-issuer" -> "Grandparent_global_certificates"; - "Grandparent_global_output" [label="Grandparent/global/output"]; + "Grandparent_global_output" [label="Grandparent/global-rg/output"]; "Grandparent_global_output" -> "Grandparent_global_decrypt-and-ingest-secrets"; "Grandparent_global_output" -> "Grandparent_global_grafana-dashboards"; "Grandparent_global_output" -> "Grandparent_global_housekeeping"; "Grandparent_global_output" -> "Grandparent_global_mirror-oc-mirror-image"; - "Grandparent_global_svcChildZone" [label="Grandparent/global/svcChildZone"]; + "Grandparent_global_svcChildZone" [label="Grandparent/global-rg/svcChildZone"]; } \ No newline at end of file From 039a7d6123a1eceaa8f7eb12c5a1be432b19833e Mon Sep 17 00:00:00 2001 From: Gerd Oberlechner Date: Tue, 7 Jul 2026 10:17:12 +0200 Subject: [PATCH 2/3] refactor(graph): extract graphBuilder to isolate build-time state Move graph construction methods (accumulate, addExternalDependencyEdges, node) from *Graph to a private *graphBuilder that embeds *Graph and holds a transient nodeIndex. This keeps the exposed Graph struct free of build-time bookkeeping while enabling O(1) node lookups during construction. --- pipelines/graph/graph.go | 67 ++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/pipelines/graph/graph.go b/pipelines/graph/graph.go index a89683b1..cb43d536 100644 --- a/pipelines/graph/graph.go +++ b/pipelines/graph/graph.go @@ -39,6 +39,25 @@ type Node struct { Parents []Identifier } +func newGraphBuilder() *graphBuilder { + return &graphBuilder{ + Graph: &Graph{ + Services: map[string]*topology.Service{}, + ResourceGroups: map[string]*types.ResourceGroupMeta{}, + resourceGroupOwners: map[string]sets.Set[string]{}, + Steps: map[string]map[string]map[string]types.Step{}, + Nodes: []Node{}, + ServiceValidationSteps: map[Identifier]types.ValidationStep{}, + }, + nodeIndex: map[Identifier]int{}, + } +} + +type graphBuilder struct { + *Graph + nodeIndex map[Identifier]int +} + // Graph holds a set of nodes, recording parent/child relationships for each, along with a set of lookup tables for // the services, resource groups, steps, etc. that the nodes represent. type Graph struct { @@ -98,20 +117,13 @@ func ForPipeline(service *topology.Service, pipeline *types.Pipeline) (*Graph, e Stamped: service.Stamped, } - graph := &Graph{ - Services: map[string]*topology.Service{}, - ResourceGroups: map[string]*types.ResourceGroupMeta{}, - resourceGroupOwners: map[string]sets.Set[string]{}, - Steps: map[string]map[string]map[string]types.Step{}, - Nodes: []Node{}, - ServiceValidationSteps: map[Identifier]types.ValidationStep{}, - } + b := newGraphBuilder() - if err := graph.accumulate(withoutChildren, map[string]*types.Pipeline{pipeline.ServiceGroup: pipeline}); err != nil { + if err := b.accumulate(withoutChildren, map[string]*types.Pipeline{pipeline.ServiceGroup: pipeline}); err != nil { return nil, err } - return graph, graph.detectCycles() + return b.Graph, b.detectCycles() } // ForEntrypoint generates a graph for all pipelines in the sub-tree of the topology identified by the entrypoint. @@ -130,17 +142,10 @@ func ForEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, roots = append(roots, root) } - graph := &Graph{ - Services: map[string]*topology.Service{}, - ResourceGroups: map[string]*types.ResourceGroupMeta{}, - resourceGroupOwners: map[string]sets.Set[string]{}, - Steps: map[string]map[string]map[string]types.Step{}, - Nodes: []Node{}, - ServiceValidationSteps: map[Identifier]types.ValidationStep{}, - } + b := newGraphBuilder() for _, root := range roots { - if err := graph.accumulate(root, pipelines); err != nil { + if err := b.accumulate(root, pipelines); err != nil { return nil, err } } @@ -149,15 +154,15 @@ func ForEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, // so the `nodesFor()` method can no longer generate bi-directional edges as it does not see other nodes to add // child relations. Instead of trying to teach `nodesFor()` how to do half of these edges, we can just do a pass // now will full context. - if err := graph.addExternalDependencyEdges(); err != nil { + if err := b.addExternalDependencyEdges(); err != nil { return nil, err } - return graph, graph.detectCycles() + return b.Graph, b.detectCycles() } // accumulate recursively traverses the service and all children, building a graph of how steps in each service depend on each other. -func (c *Graph) accumulate(service *topology.Service, pipelines map[string]*types.Pipeline) error { +func (c *graphBuilder) accumulate(service *topology.Service, pipelines map[string]*types.Pipeline) error { if _, alreadyRecorded := c.Services[service.ServiceGroup]; alreadyRecorded { return fmt.Errorf("service group %s already recorded", service.ServiceGroup) } @@ -200,7 +205,10 @@ func (c *Graph) accumulate(service *topology.Service, pipelines map[string]*type } c.Steps[service.ServiceGroup] = steps maps.Copy(c.ServiceValidationSteps, serviceValidationSteps) - c.Nodes = append(c.Nodes, nodes...) + for _, n := range nodes { + c.nodeIndex[n.Identifier] = len(c.Nodes) + c.Nodes = append(c.Nodes, n) + } var leaves []Identifier for _, node := range nodes { @@ -288,16 +296,15 @@ func (c *Graph) lookup(node Identifier) (*topology.Service, *types.ResourceGroup return svc, resourceGroup, step, nil } -func (c *Graph) node(id Identifier) (int, error) { - for i, node := range c.Nodes { - if node.ServiceGroup == id.ServiceGroup && node.ResourceGroup == id.ResourceGroup && node.Step == id.Step { - return i, nil - } +func (c *graphBuilder) node(id Identifier) (int, error) { + idx, ok := c.nodeIndex[id] + if !ok { + return 0, fmt.Errorf("node %s not found", id) } - return 0, fmt.Errorf("node %s not found", id) + return idx, nil } -func (c *Graph) addExternalDependencyEdges() error { +func (c *graphBuilder) addExternalDependencyEdges() error { for i, node := range c.Nodes { step, ok := c.Steps[node.ServiceGroup][node.ResourceGroup][node.Step] if !ok { From 40c92763218a9df9c4b250f04c86bb92bfbe5cce Mon Sep 17 00:00:00 2001 From: Gerd Oberlechner Date: Tue, 7 Jul 2026 10:54:47 +0200 Subject: [PATCH 3/3] feat(graph): support multi-stamp execution graphs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stamp-aware graph construction: when a service is marked as stamped in the topology, accumulate() expands it once per stamp during a single tree walk. Unstamped services are processed once. Key changes: - Stamp type wraps stamp identity with Unstamped sentinel and IsSet(). - ForEntrypoints accepts stamp-keyed pipelines (map[Stamp]map[string]*Pipeline). Non-stamped callers use ForEntrypoint which wraps pipelines as {Unstamped: pipelines}. - nodesFor bakes stamp into all Identifiers at creation (nodes, edges, steps, validation steps, resource group keys). - Leaf-to-root wiring between services is stamp-scoped: stamped parent leaves connect only to same-stamp child roots. Unstamped parent leaves fan out to all stamps. - External dependency resolution inherits stamp from the declaring node when the target service is stamped; unstamped targets keep Unstamped. - ResourceGroupKey introduced to key resource groups by (Stamp, Name). - Service.Stamped changed to *bool — explicit false preserved and validated, distinguishing "unset" from "intentionally not stamped". --- pipelines/graph/graph.go | 601 +++++++---- pipelines/graph/graph_stamped_test.go | 970 ++++++++++++++++++ pipelines/graph/graph_test.go | 48 + .../zz_fixture_TestStampedEntrypointDOT.dot | 36 + pipelines/topology/types.go | 28 +- pipelines/topology/types_test.go | 103 +- 6 files changed, 1591 insertions(+), 195 deletions(-) create mode 100644 pipelines/graph/graph_stamped_test.go create mode 100644 pipelines/graph/testdata/zz_fixture_TestStampedEntrypointDOT.dot diff --git a/pipelines/graph/graph.go b/pipelines/graph/graph.go index cb43d536..42dbe255 100644 --- a/pipelines/graph/graph.go +++ b/pipelines/graph/graph.go @@ -15,14 +15,47 @@ import ( "github.com/Azure/ARO-Tools/pipelines/types" ) -// Dependency records a dependency on a step in a particular service group and resource group. +// Stamp identifies a stamp within a multi-stamp execution graph. Use NewStamp to create a set +// stamp value and Unstamped for unstamped contexts. Callers should use IsSet rather than comparing +// against the zero value directly. +type Stamp struct { + value string +} + +// Unstamped returns the zero value representing an unstamped context. +func Unstamped() Stamp { return Stamp{} } + +// NewStamp creates a Stamp from a non-empty string. It returns an error if the value is empty. +func NewStamp(v string) (Stamp, error) { + if len(v) == 0 { + return Stamp{}, fmt.Errorf("stamp value must not be empty") + } + return Stamp{value: v}, nil +} + +// IsSet reports whether the stamp represents a real stamp value rather than the unstamped zero value. +func (s Stamp) IsSet() bool { return len(s.value) > 0 } + +func (s Stamp) String() string { return s.value } + +// Identifier records a dependency on a step in a particular service group and resource group. // This is the minimum amount of precision required to identify a step in a multi-pipeline execution environment. +// Stamp is set by callers that expand stamped services into multiple graph copies — it distinguishes +// nodes that share the same ServiceGroup/ResourceGroup/Step but belong to different stamps. type Identifier struct { + Stamp Stamp ServiceGroup string types.StepDependency } +func (i Identifier) ResourceGroupKey() ResourceGroupKey { + return ResourceGroupKey{Stamp: i.Stamp, Name: i.ResourceGroup} +} + func (i Identifier) String() string { + if i.Stamp.IsSet() { + return fmt.Sprintf("%s/%s/%s (stamp=%s)", i.ServiceGroup, i.ResourceGroup, i.Step, i.Stamp) + } return fmt.Sprintf("%s/%s/%s", i.ServiceGroup, i.ResourceGroup, i.Step) } @@ -30,7 +63,7 @@ func (i Identifier) String() string { // complex data, pointers to the underlying structures needed to execute the steps, etc. Such a structure helps to // make operations that produce or operate over these nodes easy to test and verify. type Node struct { - // This embedded Dependency defines the identifier for this node. + // This embedded Identifier defines the identifier for this node. Identifier // Children contains the direct children (not further descendants) of this node. @@ -39,13 +72,21 @@ type Node struct { Parents []Identifier } +// ResourceGroupKey identifies a resource group within a graph. Stamp is Unstamped for +// unstamped resource groups. Callers that expand stamped services set Stamp to +// distinguish per-stamp resource group metadata that shares the same logical Name. +type ResourceGroupKey struct { + Stamp Stamp + Name string +} + func newGraphBuilder() *graphBuilder { return &graphBuilder{ Graph: &Graph{ Services: map[string]*topology.Service{}, - ResourceGroups: map[string]*types.ResourceGroupMeta{}, - resourceGroupOwners: map[string]sets.Set[string]{}, - Steps: map[string]map[string]map[string]types.Step{}, + ResourceGroups: map[ResourceGroupKey]*types.ResourceGroupMeta{}, + resourceGroupOwners: map[ResourceGroupKey]sets.Set[string]{}, + Steps: map[Identifier]types.Step{}, Nodes: []Node{}, ServiceValidationSteps: map[Identifier]types.ValidationStep{}, }, @@ -53,6 +94,8 @@ func newGraphBuilder() *graphBuilder { } } +// graphBuilder holds transient build-time state (like nodeIndex) that should not +// persist on the final Graph. Call build() to discard the builder and return the Graph. type graphBuilder struct { *Graph nodeIndex map[Identifier]int @@ -64,17 +107,15 @@ type Graph struct { // Services is a lookup table of services by name (the service group). Services map[string]*topology.Service - // ResourceGroups is a lookup table of resource group by name. We flatten the hierarchy of resource groups to - // not require re-writing the dependency references in every step. Topologies that define more than one unique resource - // group with the same identifier are disallowed. - ResourceGroups map[string]*types.ResourceGroupMeta + // ResourceGroups is a lookup table of resource groups keyed by stamp and logical name. + // Unstamped resource groups use Unstamped. + ResourceGroups map[ResourceGroupKey]*types.ResourceGroupMeta // Subscription is an optional set of metadata required for subscription provisioning. Subscription *Subscription - // Steps is a lookup table of service group -> resource group -> step name. Steps are *not* flattened, keeping record - // of provenance and allowing step names to be kept short, unique only within their resource group. - Steps map[string]map[string]map[string]types.Step + // Steps is a lookup table keyed by Identifier. + Steps map[Identifier]types.Step // Nodes records every step, and the parent/child relationships between them. Nodes []Node @@ -83,7 +124,13 @@ type Graph struct { ServiceValidationSteps map[Identifier]types.ValidationStep // resourceGroupOwners tracks which service groups have registered each resource group (internal book-keeping). - resourceGroupOwners map[string]sets.Set[string] + resourceGroupOwners map[ResourceGroupKey]sets.Set[string] +} + +// GetStep returns the step for a node, using the node's Stamp field to select per-stamp steps. +func (c *Graph) GetStep(node Identifier) (types.Step, bool) { + s, exists := c.Steps[node] + return s, exists } // Subscription holds the metadata required to handle subscription provisioning for an execution graph. @@ -106,6 +153,13 @@ type edge struct { from, to Identifier } +// stampIteration pairs a stamp identifier with the pipeline to process for that stamp. +// Unstamped services use a single iteration with Unstamped. +type stampIteration struct { + stamp Stamp + pipeline *types.Pipeline +} + // ForPipeline generates a graph for one pipeline, processing all steps therein to determine dependencies between them. func ForPipeline(service *topology.Service, pipeline *types.Pipeline) (*Graph, error) { withoutChildren := &topology.Service{ @@ -114,25 +168,68 @@ func ForPipeline(service *topology.Service, pipeline *types.Pipeline) (*Graph, e PipelinePath: service.PipelinePath, Children: nil, // explicitly omitted to generate graph for one pipeline only Metadata: service.Metadata, + Stamped: nil, // one pipeline in, nothing to stamp-multiply + } + + graphBuilder := newGraphBuilder() + + stampPipelines := map[Stamp]map[string]*types.Pipeline{ + Unstamped(): {pipeline.ServiceGroup: pipeline}, + } + if err := graphBuilder.accumulate(withoutChildren, stampPipelines); err != nil { + return nil, err + } + + return graphBuilder.Graph, graphBuilder.detectCycles() +} + +// ForStampedPipeline generates a graph for a single service group, expanding stamped services once per stamp. +// Like ForPipeline it strips children so the graph covers exactly one pipeline, but unlike ForPipeline it +// preserves the service's Stamped flag and accepts per-stamp pipelines. Unstamped services produce a single +// set of nodes with Unstamped; stamped services produce N copies — one per stamp with IsSet() == true. +func ForStampedPipeline(service *topology.Service, stampPipelines map[Stamp]map[string]*types.Pipeline) (*Graph, error) { + withoutChildren := &topology.Service{ + ServiceGroup: service.ServiceGroup, + Purpose: service.Purpose, + PipelinePath: service.PipelinePath, + Children: nil, + Metadata: service.Metadata, Stamped: service.Stamped, } - b := newGraphBuilder() + graphBuilder := newGraphBuilder() - if err := b.accumulate(withoutChildren, map[string]*types.Pipeline{pipeline.ServiceGroup: pipeline}); err != nil { + if err := graphBuilder.accumulate(withoutChildren, stampPipelines); err != nil { return nil, err } - return b.Graph, b.detectCycles() + return graphBuilder.Graph, graphBuilder.detectCycles() } // ForEntrypoint generates a graph for all pipelines in the sub-tree of the topology identified by the entrypoint. +// Convenience wrapper around ForEntrypoints for a single entrypoint. func ForEntrypoint(topo *topology.Topology, entrypoint *topology.Entrypoint, pipelines map[string]*types.Pipeline) (*Graph, error) { return ForEntrypoints(topo, []*topology.Entrypoint{entrypoint}, pipelines) } // ForEntrypoints generates a graph for all pipelines in the sub-trees of the topology identified by the entrypoints. +// Stamped services in the topology are not expanded — each appears exactly once with Unstamped on its +// nodes. The resulting graph has one set of nodes per service, making it suitable for contexts where stamp expansion +// is handled by the runtime (e.g. EV2 rollout specs) rather than the graph itself. func ForEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, pipelines map[string]*types.Pipeline) (*Graph, error) { + return forEntrypoints(topo, entrypoints, map[Stamp]map[string]*types.Pipeline{Unstamped(): pipelines}) +} + +// ForStampedEntrypoints generates a graph for all pipelines in the sub-trees of the topology identified by the +// entrypoints, expanding stamped services once per stamp. Each stamped service produces N copies of its nodes — +// one per stamp with IsSet() == true in stampPipelines — with each copy carrying a distinct Stamp on its identifiers. +// Unstamped services appear once with Unstamped. The resulting graph is suitable for contexts where the graph +// itself drives per-stamp execution (e.g. templatize concurrent stamp rollouts). +func ForStampedEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, stampPipelines map[Stamp]map[string]*types.Pipeline) (*Graph, error) { + return forEntrypoints(topo, entrypoints, stampPipelines) +} + +func forEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, stampPipelines map[Stamp]map[string]*types.Pipeline) (*Graph, error) { var roots []*topology.Service for _, entrypoint := range entrypoints { root, err := topo.Lookup(entrypoint.Identifier) @@ -142,156 +239,267 @@ func ForEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, roots = append(roots, root) } - b := newGraphBuilder() + graphBuilder := newGraphBuilder() for _, root := range roots { - if err := b.accumulate(root, pipelines); err != nil { + if err := graphBuilder.accumulate(root, stampPipelines); err != nil { return nil, err } } // External step dependencies break the nice separation between nodes of one pipeline and the rest of the graph, - // so the `nodesFor()` method can no longer generate bi-directional edges as it does not see other nodes to add - // child relations. Instead of trying to teach `nodesFor()` how to do half of these edges, we can just do a pass - // now will full context. - if err := b.addExternalDependencyEdges(); err != nil { + // so the nodesFor() method can no longer generate bi-directional edges as it does not see other nodes to add + // child relations. Instead of trying to teach nodesFor() how to do half of these edges, we can do a pass + // now with full context. + if err := graphBuilder.addExternalDependencyEdges(); err != nil { return nil, err } - return b.Graph, b.detectCycles() + return graphBuilder.Graph, graphBuilder.detectCycles() } -// accumulate recursively traverses the service and all children, building a graph of how steps in each service depend on each other. -func (c *graphBuilder) accumulate(service *topology.Service, pipelines map[string]*types.Pipeline) error { +// accumulate recursively traverses the service and all children, building a graph of how steps in each +// service depend on each other. Stamped services are expanded once per stamp; unstamped services once. +func (c *graphBuilder) accumulate(service *topology.Service, stampPipelines map[Stamp]map[string]*types.Pipeline) error { if _, alreadyRecorded := c.Services[service.ServiceGroup]; alreadyRecorded { return fmt.Errorf("service group %s already recorded", service.ServiceGroup) } - if _, alreadyRecorded := c.Steps[service.ServiceGroup]; alreadyRecorded { - return fmt.Errorf("steps already recorded for service %s", service.ServiceGroup) + c.Services[service.ServiceGroup] = service + + iterations, err := resolveIterations(service, stampPipelines) + if err != nil { + return err } - c.Services[service.ServiceGroup] = service - c.Steps[service.ServiceGroup] = map[string]map[string]types.Step{} + // Leaf nodes are collected per stamp so that inter-service wiring connects only matching stamps. + allLeaves := map[Stamp][]Identifier{} + for _, iter := range iterations { + leaves, err := c.accumulateIteration(service.ServiceGroup, iter) + if err != nil { + return err + } + allLeaves[iter.stamp] = leaves + } - pipeline, exists := pipelines[service.ServiceGroup] - if !exists { - return fmt.Errorf("pipeline for service %s not found", service.ServiceGroup) + // Wire inter-service edges: connect this service's leaves to child service roots. + for _, child := range service.Children { + if err := c.accumulate(&child, stampPipelines); err != nil { + return err + } + if err := c.wireInterServiceEdges(service, &child, allLeaves); err != nil { + return err + } } - resourceGroups, subscription, steps, serviceValidationSteps, nodes, err := nodesFor(pipeline) - if err != nil { - return fmt.Errorf("failed to generate graph for pipeline %s: %v", service.ServiceGroup, err) + + return nil +} + +// resolveIterations determines which stamp/pipeline pairs to process for a service. +// Unstamped services run once with Unstamped. Stamped services expand once per +// stamp key where IsSet() is true. +func resolveIterations(service *topology.Service, stampPipelines map[Stamp]map[string]*types.Pipeline) ([]stampIteration, error) { + // Unstamped services use the same pipeline regardless of stamp — grab from any entry. + if !service.IsStamped() { + for _, pipelines := range stampPipelines { + if p, exists := pipelines[service.ServiceGroup]; exists { + return []stampIteration{{pipeline: p}}, nil + } + } + return nil, fmt.Errorf("pipeline for service %s not found", service.ServiceGroup) } - for name, group := range resourceGroups { - if other, alreadyRecorded := c.ResourceGroups[name]; alreadyRecorded { - if !resourceGroupMetaEqual(group, other) { - existingOwners := sets.List(c.resourceGroupOwners[name]) - slices.Sort(existingOwners) - return fmt.Errorf("resource group %s already recorded with different step meta (existing services: %s, new service: %s), diff: %v", name, strings.Join(existingOwners, ", "), service.ServiceGroup, cmp.Diff(group, other)) - } else { - // Same metadata, just add this service as an owner if not already present - c.resourceGroupOwners[name].Insert(service.ServiceGroup) + + var stamps []Stamp + for stamp := range stampPipelines { + if stamp.IsSet() { + stamps = append(stamps, stamp) + } + } + slices.SortFunc(stamps, func(a, b Stamp) int { + return strings.Compare(a.String(), b.String()) + }) + + // No set stamps: fall back to single iteration without expansion. + if len(stamps) == 0 { + for _, pipelines := range stampPipelines { + if p, exists := pipelines[service.ServiceGroup]; exists { + return []stampIteration{{pipeline: p}}, nil } - } else { - // First time recording this resource group - c.ResourceGroups[name] = group - c.resourceGroupOwners[name] = sets.New(service.ServiceGroup) } + return nil, fmt.Errorf("pipeline for service %s not found", service.ServiceGroup) } - if subscription != nil { - if c.Subscription != nil { - return fmt.Errorf("subscription provisioning already recorded for %s/%s, cannot add another for %s/%s", c.Subscription.ServiceGroup, c.Subscription.ResourceGroup, subscription.ServiceGroup, subscription.ResourceGroup) + + var iterations []stampIteration + for _, stamp := range stamps { + pipeline, exists := stampPipelines[stamp][service.ServiceGroup] + if !exists { + return nil, fmt.Errorf("pipeline for service %s not found in stamp %s", service.ServiceGroup, stamp) } + iterations = append(iterations, stampIteration{stamp: stamp, pipeline: pipeline}) + } + return iterations, nil +} + +// accumulateIteration processes one stamp iteration: generates nodes, registers resource groups, +// steps, and metadata, and returns the leaf nodes for inter-service wiring. +func (c *graphBuilder) accumulateIteration(serviceGroup string, iter stampIteration) ([]Identifier, error) { + resourceGroups, subscription, steps, serviceValidationSteps, nodes, err := nodesFor(iter.pipeline, iter.stamp) + if err != nil { + return nil, fmt.Errorf("failed to generate graph for pipeline %s: %v", serviceGroup, err) + } + + if err := c.registerResourceGroups(serviceGroup, iter.stamp, resourceGroups); err != nil { + return nil, err + } + + if subscription != nil && c.Subscription != nil { + return nil, fmt.Errorf("subscription provisioning already recorded for %s/%s, cannot add another for %s/%s", c.Subscription.ServiceGroup, c.Subscription.ResourceGroup, subscription.ServiceGroup, subscription.ResourceGroup) + } + if subscription != nil { c.Subscription = subscription } - c.Steps[service.ServiceGroup] = steps + + // Register steps and validation steps with stamp-qualified identifiers. + for rg, stepMap := range steps { + for stepName, step := range stepMap { + c.Steps[Identifier{Stamp: iter.stamp, ServiceGroup: serviceGroup, StepDependency: types.StepDependency{ResourceGroup: rg, Step: stepName}}] = step + } + } maps.Copy(c.ServiceValidationSteps, serviceValidationSteps) for _, n := range nodes { c.nodeIndex[n.Identifier] = len(c.Nodes) c.Nodes = append(c.Nodes, n) } + return c.findLeaves(nodes) +} + +// registerResourceGroups records resource groups, detecting conflicts across services and stamps. +func (c *graphBuilder) registerResourceGroups(serviceGroup string, stamp Stamp, resourceGroups map[string]*types.ResourceGroupMeta) error { + for name, group := range resourceGroups { + key := ResourceGroupKey{Stamp: stamp, Name: name} + other, alreadyRecorded := c.ResourceGroups[key] + if alreadyRecorded && !resourceGroupMetaEqual(group, other) { + existingOwners := sets.List(c.resourceGroupOwners[key]) + return fmt.Errorf("resource group %s already recorded with different step meta (existing services: %s, new service: %s), diff: %v", name, strings.Join(existingOwners, ", "), serviceGroup, cmp.Diff(group, other)) + } + if !alreadyRecorded { + c.ResourceGroups[key] = group + c.resourceGroupOwners[key] = sets.New[string]() + } + c.resourceGroupOwners[key].Insert(serviceGroup) + } + return nil +} + +// findLeaves identifies leaf nodes for a stamp iteration — these will become parents of child service roots. +// A node is a leaf when it is considered for service group completion and none of its children are. +func (c *graphBuilder) findLeaves(nodes []Node) ([]Identifier, error) { var leaves []Identifier for _, node := range nodes { _, _, step, err := c.lookup(node.Identifier) if err != nil { - return fmt.Errorf("failed to lookup node: %v", err) + return nil, fmt.Errorf("failed to lookup node: %v", err) } - if len(node.Children) == 0 { - if step.ConsideredForServiceGroupCompletion() { - leaves = append(leaves, node.Identifier) - } - } else if step.ConsideredForServiceGroupCompletion() { - ignoredLeaves := 0 - for _, child := range node.Children { - _, _, childStep, err := c.lookup(child) - if err != nil { - return fmt.Errorf("failed to lookup node: %v", err) - } - if !childStep.ConsideredForServiceGroupCompletion() { - ignoredLeaves++ - } - } - if ignoredLeaves == len(node.Children) { - leaves = append(leaves, node.Identifier) - } + if !step.ConsideredForServiceGroupCompletion() { + continue + } + has, err := c.hasCompletionChild(node) + if err != nil { + return nil, err } + if has { + continue + } + leaves = append(leaves, node.Identifier) } + return leaves, nil +} - for _, child := range service.Children { - if err := c.accumulate(&child, pipelines); err != nil { - return err +func (c *graphBuilder) hasCompletionChild(node Node) (bool, error) { + for _, child := range node.Children { + _, _, childStep, err := c.lookup(child) + if err != nil { + return false, fmt.Errorf("failed to lookup child node %s of %s: %w", child, node.Identifier, err) + } + if childStep.ConsideredForServiceGroupCompletion() { + return true, nil } + } + return false, nil +} - // The data we're using to build this graph come in two levels of granularity: - // - specific, intra-service step relationships defined in a pipeline - // - granular, inter-service relationships defined in the topology - // The above call to accumulate() will have build a sub-graph of step nodes for the specific child service, - // which we now need to decorate to record that all steps in that child depend on the parent service. - // There is no defined "end" to a pipeline, nor a "start", as each service may itself be a forest - having - // many roots and many leaves. Therefore, the simplest approach here is to record that every root node - // of the child depends on all the leaf nodes of the parent service, and vice versa. - - // First, find the root nodes for the child service, which accumulate() will have placed in the graph. - // Record those roots for later use and mark them as being children of the leaves of the parent. - var roots []Identifier - for i, node := range c.Nodes { - if node.ServiceGroup == child.ServiceGroup && len(node.Parents) == 0 { - // accumulate() did not divulge the list of root nodes for that specific child, but we can find them - roots = append(roots, node.Identifier) - - // our topology allows each service to depend on one and only one parent, so we know that - // a) it's safe to determine that `len(node.Parents) == 0` identifies a root node for that service - // b) this is the only time that any actor will add parents to this root node - c.Nodes[i].Parents = append(c.Nodes[i].Parents, leaves...) +// wireInterServiceEdges connects parent service leaves to child service roots. +// +// The data we're using to build this graph come in two levels of granularity: +// - specific, intra-service step relationships defined in a pipeline +// - granular, inter-service relationships defined in the topology +// +// accumulate() will have built a sub-graph of step nodes for the specific child service, +// which we now need to decorate to record that all steps in that child depend on the parent service. +// There is no defined "end" to a pipeline, nor a "start", as each service may itself be a forest - having +// many roots and many leaves. Therefore, the simplest approach here is to record that every root node +// of the child depends on all the leaf nodes of the parent service, and vice versa. +// +// When stamps are involved, wiring is stamp-scoped: stamp-1 leaves connect to stamp-1 roots only. +// Unstamped leaves (Unstamped) connect to all child roots that share Unstamped or fan out +// to all stamps when the child is stamped and the parent is not. +func (c *graphBuilder) wireInterServiceEdges(parent *topology.Service, child *topology.Service, allLeaves map[Stamp][]Identifier) error { + for parentStamp, leaves := range allLeaves { + roots := c.findChildRoots(parent, child, parentStamp) + rootList := roots.UnsortedList() + slices.SortFunc(rootList, CompareDependencies) + + for _, root := range rootList { + idx, err := c.node(root) + if err != nil { + return fmt.Errorf("failed to find root node: %v", err) } + c.Nodes[idx].Parents = append(c.Nodes[idx].Parents, leaves...) } - // Second, using the identifiers in `leaves`, find the leaf nodes in the graph and mark them as having - // the roots of the child service as children. - for i, node := range c.Nodes { - for _, leaf := range leaves { - if node.ServiceGroup == leaf.ServiceGroup && node.ResourceGroup == leaf.ResourceGroup && node.Step == leaf.Step { - c.Nodes[i].Children = append(c.Nodes[i].Children, roots...) - } + for _, leaf := range leaves { + idx, err := c.node(leaf) + if err != nil { + return fmt.Errorf("failed to find leaf node: %v", err) } + c.Nodes[idx].Children = append(c.Nodes[idx].Children, rootList...) } } - return nil } +// findChildRoots returns identifiers of root nodes (no parents) for the child service. +// Our topology allows each service to depend on one and only one parent, so len(node.Parents) == 0 +// safely identifies root nodes, and this is the only time any actor will add parents to these roots. +// When both parent and child are stamped, only matching-stamp roots are returned. +func (c *graphBuilder) findChildRoots(parent, child *topology.Service, parentStamp Stamp) sets.Set[Identifier] { + roots := sets.New[Identifier]() + for _, node := range c.Nodes { + if node.ServiceGroup != child.ServiceGroup || len(node.Parents) != 0 { + continue + } + // When both parent and child are stamped, only wire matching stamps. + // When parent is unstamped, all stamped child roots get these leaves (fan-out). + if parent.IsStamped() && child.IsStamped() && node.Stamp != parentStamp { + continue + } + roots.Insert(node.Identifier) + } + return roots +} + func (c *Graph) lookup(node Identifier) (*topology.Service, *types.ResourceGroupMeta, types.Step, error) { svc, exists := c.Services[node.ServiceGroup] if !exists { return nil, nil, nil, fmt.Errorf("service %s does not exist", node.ServiceGroup) } - resourceGroup, exists := c.ResourceGroups[node.ResourceGroup] + resourceGroup, exists := c.ResourceGroups[node.ResourceGroupKey()] if !exists { - return nil, nil, nil, fmt.Errorf("resource group %s does not exist", node.ResourceGroup) + return nil, nil, nil, fmt.Errorf("resource group %s for node %s does not exist", node.ResourceGroup, node) } - step, exists := c.Steps[node.ServiceGroup][node.ResourceGroup][node.Step] + step, exists := c.GetStep(node) if !exists { - return nil, nil, nil, fmt.Errorf("step %s/%s/%s does not exist", node.ServiceGroup, node.ResourceGroup, node.Step) + return nil, nil, nil, fmt.Errorf("step %s does not exist", node) } return svc, resourceGroup, step, nil } @@ -306,9 +514,9 @@ func (c *graphBuilder) node(id Identifier) (int, error) { func (c *graphBuilder) addExternalDependencyEdges() error { for i, node := range c.Nodes { - step, ok := c.Steps[node.ServiceGroup][node.ResourceGroup][node.Step] + step, ok := c.GetStep(node.Identifier) if !ok { - return fmt.Errorf("step %s/%s/%s not found", node.ServiceGroup, node.ResourceGroup, node.Step) + return fmt.Errorf("step %s not found", node.Identifier) } external := step.ExternalDependencies() if len(external) == 0 { @@ -322,6 +530,16 @@ func (c *graphBuilder) addExternalDependencyEdges() error { Step: dep.Step, }, } + // Unstamped services cannot declare external dependencies on stamped services — + // there is no single stamp to resolve to. When both sides are stamped, resolve + // to the same stamp: stamp-1 work depends on stamp-1 of the target. + if targetService := c.Services[dep.ServiceGroup]; targetService != nil && targetService.IsStamped() { + sourceService := c.Services[node.ServiceGroup] + if sourceService == nil || !sourceService.IsStamped() { + return fmt.Errorf("unstamped node %s has an external dependency on stamped service %s — unstamped services cannot depend on stamped services", node.Identifier, dep.ServiceGroup) + } + parent.Stamp = node.Stamp + } parentNodeIdx, err := c.node(parent) if err != nil { return err @@ -334,15 +552,15 @@ func (c *graphBuilder) addExternalDependencyEdges() error { node.Parents = append(node.Parents, parent) } - slices.SortFunc(node.Children, CompareDependencies) - node.Children = slices.Compact(node.Children) + slices.SortFunc(node.Parents, CompareDependencies) + node.Parents = slices.Compact(node.Parents) c.Nodes[i] = node } return nil } // nodesFor transforms a pipeline to the list of nodes and lookup tables required in a graph -func nodesFor(pipeline *types.Pipeline) ( +func nodesFor(pipeline *types.Pipeline, stamp Stamp) ( map[string]*types.ResourceGroupMeta, *Subscription, map[string]map[string]types.Step, @@ -350,18 +568,16 @@ func nodesFor(pipeline *types.Pipeline) ( []Node, error, ) { - // first, create a registry of steps by their identifier (resource group name, step name) - // and resource groups by name stepsByResourceGroupAndName := map[string]map[string]types.Step{} serviceValidationSteps := map[Identifier]types.ValidationStep{} resourceGroupsByName := map[string]*types.ResourceGroupMeta{} var subscription *Subscription for _, rg := range pipeline.ResourceGroups { resourceGroupsByName[rg.Name] = rg.ResourceGroupMeta + if rg.SubscriptionProvisioning != nil && subscription != nil { + return nil, nil, nil, nil, nil, fmt.Errorf("multiple subscriptions found for pipeline %s", pipeline.ServiceGroup) + } if rg.SubscriptionProvisioning != nil { - if subscription != nil { - return nil, nil, nil, nil, nil, fmt.Errorf("multiple subscriptions found for pipeline %s", pipeline.ServiceGroup) - } subscription = &Subscription{ ServiceGroup: pipeline.ServiceGroup, ResourceGroup: rg.Name, @@ -374,6 +590,7 @@ func nodesFor(pipeline *types.Pipeline) ( } for _, step := range rg.ValidationSteps { serviceValidationSteps[Identifier{ + Stamp: stamp, ServiceGroup: pipeline.ServiceGroup, StepDependency: types.StepDependency{ ResourceGroup: rg.Name, @@ -383,7 +600,6 @@ func nodesFor(pipeline *types.Pipeline) ( } } - // next, create an adjacency list of edges between these nodes var stepDependencies []edge for _, rg := range pipeline.ResourceGroups { for _, step := range rg.Steps { @@ -394,6 +610,7 @@ func nodesFor(pipeline *types.Pipeline) ( for _, dep := range dependsOn { stepDependencies = append(stepDependencies, edge{ from: Identifier{ + Stamp: stamp, ServiceGroup: pipeline.ServiceGroup, StepDependency: types.StepDependency{ ResourceGroup: dep.ResourceGroup, @@ -401,6 +618,7 @@ func nodesFor(pipeline *types.Pipeline) ( }, }, to: Identifier{ + Stamp: stamp, ServiceGroup: pipeline.ServiceGroup, StepDependency: types.StepDependency{ ResourceGroup: rg.Name, @@ -419,29 +637,29 @@ func nodesFor(pipeline *types.Pipeline) ( return CompareDependencies(a.to, b.to) }) + childrenOf := map[Identifier][]Identifier{} + parentsOf := map[Identifier][]Identifier{} + for _, e := range stepDependencies { + childrenOf[e.from] = append(childrenOf[e.from], e.to) + parentsOf[e.to] = append(parentsOf[e.to], e.from) + } + var nodes []Node for resourceGroup, steps := range stepsByResourceGroupAndName { for stepName := range steps { - node := Node{ - Identifier: Identifier{ - ServiceGroup: pipeline.ServiceGroup, - StepDependency: types.StepDependency{ - ResourceGroup: resourceGroup, - Step: stepName, - }, + id := Identifier{ + Stamp: stamp, + ServiceGroup: pipeline.ServiceGroup, + StepDependency: types.StepDependency{ + ResourceGroup: resourceGroup, + Step: stepName, }, - Children: []Identifier{}, - Parents: []Identifier{}, } - for _, edge := range stepDependencies { - if edge.to.ServiceGroup == pipeline.ServiceGroup && edge.to.ResourceGroup == resourceGroup && edge.to.Step == stepName { - node.Parents = append(node.Parents, edge.from) - } - if edge.from.ServiceGroup == pipeline.ServiceGroup && edge.from.ResourceGroup == resourceGroup && edge.from.Step == stepName { - node.Children = append(node.Children, edge.to) - } - } - nodes = append(nodes, node) + nodes = append(nodes, Node{ + Identifier: id, + Children: childrenOf[id], + Parents: parentsOf[id], + }) } } @@ -453,6 +671,9 @@ func nodesFor(pipeline *types.Pipeline) ( } func CompareDependencies(a, b Identifier) int { + if comparison := strings.Compare(a.Stamp.String(), b.Stamp.String()); comparison != 0 { + return comparison + } if comparison := strings.Compare(a.ServiceGroup, b.ServiceGroup); comparison != 0 { return comparison } @@ -479,17 +700,18 @@ func resourceGroupMetaEqual(a, b *types.ResourceGroupMeta) bool { if len(a.ExecutionConstraints) != len(b.ExecutionConstraints) { return false } - for i := 0; i < len(a.ExecutionConstraints); i++ { - if a.ExecutionConstraints[i].Singleton != b.ExecutionConstraints[i].Singleton { + for i, ac := range a.ExecutionConstraints { + bc := b.ExecutionConstraints[i] + if ac.Singleton != bc.Singleton { return false } - if !sets.New(a.ExecutionConstraints[i].Clouds...).Equal(sets.New(b.ExecutionConstraints[i].Clouds...)) { + if !sets.New(ac.Clouds...).Equal(sets.New(bc.Clouds...)) { return false } - if !sets.New(a.ExecutionConstraints[i].Environments...).Equal(sets.New(b.ExecutionConstraints[i].Environments...)) { + if !sets.New(ac.Environments...).Equal(sets.New(bc.Environments...)) { return false } - if !sets.New(a.ExecutionConstraints[i].Regions...).Equal(sets.New(b.ExecutionConstraints[i].Regions...)) { + if !sets.New(ac.Regions...).Equal(sets.New(bc.Regions...)) { return false } } @@ -499,40 +721,32 @@ func resourceGroupMetaEqual(a, b *types.ResourceGroupMeta) bool { // detectCycles runs a depth-first traversal of the tree, starting at every node, to detect cycles func (c *Graph) detectCycles() error { + nodesByID := make(map[Identifier]Node, len(c.Nodes)) for _, node := range c.Nodes { - seen := []Identifier{ - node.Identifier, - } - if err := traverse(node, c.Nodes, seen); err != nil { + nodesByID[node.Identifier] = node + } + for _, node := range c.Nodes { + if err := traverse(node, nodesByID, []Identifier{node.Identifier}); err != nil { return err } } return nil } -func traverse(node Node, all []Node, seen []Identifier) error { +func traverse(node Node, nodesByID map[Identifier]Node, seen []Identifier) error { for _, child := range node.Children { - for _, previous := range seen { - if previous == child { - var cycle []string - for _, i := range seen { - cycle = append(cycle, fmt.Sprintf("%s/%s", i.ResourceGroup, i.Step)) - } - return fmt.Errorf("cycle detected, reached %s/%s via %s", child.ResourceGroup, child.Step, strings.Join(cycle, " -> ")) + if slices.Contains(seen, child) { + var cycle []string + for _, i := range seen { + cycle = append(cycle, i.String()) } + return fmt.Errorf("cycle detected, reached %s via %s", child, strings.Join(cycle, " -> ")) } - chain := seen[:] - chain = append(chain, child) - var childNode Node - for _, candidate := range all { - if candidate.ServiceGroup == child.ServiceGroup && candidate.ResourceGroup == child.ResourceGroup && candidate.Step == child.Step { - childNode = candidate - } - } - if childNode.ServiceGroup == "" { - return fmt.Errorf("could not find child node %s/%s - programmer error", child.ResourceGroup, child.Step) + childNode, found := nodesByID[child] + if !found { + return fmt.Errorf("could not find child node %s - programmer error", child) } - if err := traverse(childNode, all, chain); err != nil { + if err := traverse(childNode, nodesByID, append(seen[:len(seen):len(seen)], child)); err != nil { return err } } @@ -547,7 +761,7 @@ const graphPrefix = `digraph regexp { const graphSuffix = `}` -// MarshalDOT marshals the graph described by the list of nodes into the DOT notation used by the graphviz library. +// MarshalDOT marshals the graph into the DOT notation used by the graphviz library. // See documentation here: https://graphviz.gitlab.io/doc/info/lang.html func MarshalDOT(g *Graph) ([]byte, error) { out := bytes.Buffer{} @@ -555,6 +769,8 @@ func MarshalDOT(g *Graph) ([]byte, error) { return nil, fmt.Errorf("failed to write graph prefix: wrote %d/%d bytes: %w", n, len(graphPrefix), err) } + stampColors := buildStampColorMap(g.Nodes) + for _, node := range g.Nodes { serviceGroup, err := shortenServiceGroup(node.ServiceGroup) if err != nil { @@ -563,7 +779,11 @@ func MarshalDOT(g *Graph) ([]byte, error) { nodeID := dotID(serviceGroup, node.Identifier) nodeLabel := dotLabel(serviceGroup, node.Identifier, g.ResourceGroups) - if _, err := fmt.Fprintf(&out, " \"%s\" [label=\"%s\"];\n", nodeID, nodeLabel); err != nil { + attrs := fmt.Sprintf("label=\"%s\"", nodeLabel) + if color, ok := stampColors[node.Stamp]; ok { + attrs += fmt.Sprintf(" style=filled fillcolor=\"%s\"", color) + } + if _, err := fmt.Fprintf(&out, " \"%s\" [%s];\n", nodeID, attrs); err != nil { return nil, err } @@ -596,16 +816,33 @@ func MarshalDOT(g *Graph) ([]byte, error) { return out.Bytes(), nil } -func dotID(shortSG string, id Identifier) string { - return fmt.Sprintf("%s_%s_%s", shortSG, id.ResourceGroup, id.Step) +var stampColorPalette = []string{ + "#B3D9FF", // light blue + "#FFD9B3", // light orange + "#B3FFB3", // light green + "#FFB3D9", // light pink + "#D9B3FF", // light purple + "#FFFFB3", // light yellow + "#B3FFFF", // light cyan + "#FFB3B3", // light red } -func dotLabel(shortSG string, id Identifier, resourceGroups map[string]*types.ResourceGroupMeta) string { - rgName := id.ResourceGroup - if rg, ok := resourceGroups[id.ResourceGroup]; ok { - rgName = rg.ResourceGroup +func buildStampColorMap(nodes []Node) map[Stamp]string { + stamps := sets.New[Stamp]() + for _, node := range nodes { + if node.Stamp.IsSet() { + stamps.Insert(node.Stamp) + } } - return fmt.Sprintf("%s/%s/%s", shortSG, rgName, id.Step) + sorted := stamps.UnsortedList() + slices.SortFunc(sorted, func(a, b Stamp) int { + return strings.Compare(a.String(), b.String()) + }) + colors := make(map[Stamp]string, len(sorted)) + for i, stamp := range sorted { + colors[stamp] = stampColorPalette[i%len(stampColorPalette)] + } + return colors } func shortenServiceGroup(serviceGroup string) (string, error) { @@ -616,3 +853,21 @@ func shortenServiceGroup(serviceGroup string) (string, error) { return strings.Join(parts[4:], "."), nil } + +func dotID(shortSG string, id Identifier) string { + if id.Stamp.IsSet() { + return fmt.Sprintf("%s_%s_%s_%s", shortSG, id.ResourceGroup, id.Step, id.Stamp) + } + return fmt.Sprintf("%s_%s_%s", shortSG, id.ResourceGroup, id.Step) +} + +func dotLabel(shortSG string, id Identifier, resourceGroups map[ResourceGroupKey]*types.ResourceGroupMeta) string { + rgName := id.ResourceGroup + if rg, ok := resourceGroups[id.ResourceGroupKey()]; ok { + rgName = rg.ResourceGroup + } + if id.Stamp.IsSet() { + return fmt.Sprintf("%s/%s/%s (stamp=%s)", shortSG, rgName, id.Step, id.Stamp) + } + return fmt.Sprintf("%s/%s/%s", shortSG, rgName, id.Step) +} diff --git a/pipelines/graph/graph_stamped_test.go b/pipelines/graph/graph_stamped_test.go new file mode 100644 index 00000000..85157a61 --- /dev/null +++ b/pipelines/graph/graph_stamped_test.go @@ -0,0 +1,970 @@ +package graph + +import ( + "slices" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Azure/ARO-Tools/pipelines/topology" + "github.com/Azure/ARO-Tools/pipelines/types" + "github.com/Azure/ARO-Tools/testutil" +) + +func ptr[T any](v T) *T { return &v } + +func mustStamp(v string) Stamp { + s, err := NewStamp(v) + if err != nil { + panic(err) + } + return s +} + +// nodesForSG filters graph nodes to those belonging to the given service group. +func nodesForSG(result *Graph, serviceGroup string) []Node { + var nodes []Node + for _, node := range result.Nodes { + if node.ServiceGroup == serviceGroup { + nodes = append(nodes, node) + } + } + return nodes +} + +// makeTopology builds a topology from a root service, registers it as the sole entrypoint, and propagates stamp flags. +func makeTopology(root topology.Service) *topology.Topology { + topo := &topology.Topology{ + Services: []topology.Service{root}, + Entrypoints: []topology.Entrypoint{{Identifier: root.ServiceGroup}}, + } + topo.PropagateStamped() + return topo +} + +// makePipeline builds a single-resource-group pipeline with auto-derived RG metadata. +func makePipeline(serviceGroup, rgName string, steps ...types.Step) *types.Pipeline { + return &types.Pipeline{ + ServiceGroup: serviceGroup, + ResourceGroups: []*types.ResourceGroup{ + { + ResourceGroupMeta: &types.ResourceGroupMeta{ + Name: rgName, + ResourceGroup: rgName, + Subscription: "sub-" + rgName, + }, + Steps: steps, + }, + }, + } +} + +// makePipelineWithRGMeta builds a single-resource-group pipeline with explicit RG metadata (e.g. per-stamp subscriptions). +func makePipelineWithRGMeta(serviceGroup string, rgMeta *types.ResourceGroupMeta, steps ...types.Step) *types.Pipeline { + return &types.Pipeline{ + ServiceGroup: serviceGroup, + ResourceGroups: []*types.ResourceGroup{ + { + ResourceGroupMeta: rgMeta, + Steps: steps, + }, + }, + } +} + +// makePipelineWithValidation builds a pipeline with both regular steps and validation steps. +func makePipelineWithValidation(serviceGroup string, rgMeta *types.ResourceGroupMeta, validationSteps []types.ValidationStep, steps ...types.Step) *types.Pipeline { + return &types.Pipeline{ + ServiceGroup: serviceGroup, + ResourceGroups: []*types.ResourceGroup{ + { + ResourceGroupMeta: rgMeta, + Steps: steps, + ValidationSteps: validationSteps, + }, + }, + } +} + +// stampSets extracts and sorts the stamp values from a slice of nodes for comparison. +func stampSets(nodes []Node) []string { + var stamps []string + for _, node := range nodes { + stamps = append(stamps, node.Stamp.String()) + } + slices.Sort(stamps) + return stamps +} + +// buildAndValidate constructs a graph via ForStampedEntrypoints and runs the validation function against it. +func buildAndValidate(t *testing.T, topo *topology.Topology, stampPipelines map[Stamp]map[string]*types.Pipeline, validate func(t *testing.T, result *Graph)) { + t.Helper() + entrypoint := &topo.Entrypoints[0] + result, err := ForStampedEntrypoints(topo, []*topology.Entrypoint{entrypoint}, stampPipelines) + require.NoError(t, err) + validate(t, result) +} + +// TestStampedUnstampedParent tests stamped graph construction with: +// +// SG.Infra (unstamped) → SG.Mgmt (stamped) +func TestStampedUnstampedParent(t *testing.T) { + deploy := &types.ShellStep{StepMeta: types.StepMeta{Name: "deploy"}} + + topo := makeTopology(topology.Service{ + ServiceGroup: "SG.Infra", Purpose: "infra", PipelinePath: "infra.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml"}, + }, + }) + + infraPipeline := makePipeline("SG.Infra", "infra-rg", deploy) + + twoStampPipelines := map[Stamp]map[string]*types.Pipeline{ + mustStamp("1"): { + "SG.Infra": infraPipeline, + "SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-1", Subscription: "sub-1"}, deploy), + }, + mustStamp("2"): { + "SG.Infra": infraPipeline, + "SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-2", Subscription: "sub-2"}, deploy), + }, + } + + testCases := []struct { + name string + stampPipelines map[Stamp]map[string]*types.Pipeline + validate func(t *testing.T, result *Graph) + }{ + { + name: "unstamped nodes appear once across stamps", + stampPipelines: twoStampPipelines, + validate: func(t *testing.T, result *Graph) { + infraNodes := nodesForSG(result, "SG.Infra") + assert.Equal(t, 1, len(infraNodes)) + assert.False(t, infraNodes[0].Stamp.IsSet()) + }, + }, + { + name: "stamped nodes appear once per stamp", + stampPipelines: twoStampPipelines, + validate: func(t *testing.T, result *Graph) { + mgmtNodes := nodesForSG(result, "SG.Mgmt") + assert.Equal(t, 2, len(mgmtNodes)) + assert.Equal(t, []string{"1", "2"}, stampSets(mgmtNodes)) + }, + }, + { + name: "stamped nodes reference unstamped parent", + stampPipelines: twoStampPipelines, + validate: func(t *testing.T, result *Graph) { + infraID := Identifier{ServiceGroup: "SG.Infra", StepDependency: types.StepDependency{ResourceGroup: "infra-rg", Step: "deploy"}} + for _, node := range nodesForSG(result, "SG.Mgmt") { + assert.Equal(t, []Identifier{infraID}, node.Parents) + } + }, + }, + { + name: "unstamped parent has one child per stamp", + stampPipelines: twoStampPipelines, + validate: func(t *testing.T, result *Graph) { + infraNode := nodesForSG(result, "SG.Infra")[0] + var childStamps []string + for _, child := range infraNode.Children { + assert.Equal(t, "SG.Mgmt", child.ServiceGroup) + childStamps = append(childStamps, child.Stamp.String()) + } + slices.Sort(childStamps) + assert.Equal(t, []string{"1", "2"}, childStamps) + }, + }, + { + name: "no stale unstamped child references on parent", + stampPipelines: twoStampPipelines, + validate: func(t *testing.T, result *Graph) { + for _, child := range nodesForSG(result, "SG.Infra")[0].Children { + if child.ServiceGroup == "SG.Mgmt" { + assert.True(t, child.Stamp.IsSet()) + } + } + }, + }, + { + name: "resource groups keyed by stamp for stamped services", + stampPipelines: twoStampPipelines, + validate: func(t *testing.T, result *Graph) { + _, ok := result.ResourceGroups[ResourceGroupKey{Name: "infra-rg"}] + assert.True(t, ok, "unstamped RG present") + + _, ok = result.ResourceGroups[ResourceGroupKey{Name: "mgmt-rg"}] + assert.False(t, ok, "no unstamped mgmt-rg") + + rg1 := result.ResourceGroups[ResourceGroupKey{Stamp: mustStamp("1"), Name: "mgmt-rg"}] + assert.Equal(t, "mgmt-rg-1", rg1.ResourceGroup) + + rg2 := result.ResourceGroups[ResourceGroupKey{Stamp: mustStamp("2"), Name: "mgmt-rg"}] + assert.Equal(t, "mgmt-rg-2", rg2.ResourceGroup) + }, + }, + { + name: "single stamp graph is valid", + stampPipelines: map[Stamp]map[string]*types.Pipeline{ + mustStamp("1"): { + "SG.Infra": infraPipeline, + "SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-1", Subscription: "sub-1"}, deploy), + }, + }, + validate: func(t *testing.T, result *Graph) { + assert.Equal(t, 2, len(result.Nodes)) + infraNode := nodesForSG(result, "SG.Infra")[0] + assert.False(t, infraNode.Stamp.IsSet()) + assert.Equal(t, 1, len(infraNode.Children)) + assert.Equal(t, mustStamp("1"), infraNode.Children[0].Stamp) + + mgmtNode := nodesForSG(result, "SG.Mgmt")[0] + assert.Equal(t, mustStamp("1"), mgmtNode.Stamp) + }, + }, + { + name: "step lookups work for stamped and unstamped", + stampPipelines: twoStampPipelines, + validate: func(t *testing.T, result *Graph) { + _, ok := result.GetStep(Identifier{ServiceGroup: "SG.Infra", StepDependency: types.StepDependency{ResourceGroup: "infra-rg", Step: "deploy"}}) + assert.True(t, ok, "unstamped step") + + _, ok = result.GetStep(Identifier{Stamp: mustStamp("1"), ServiceGroup: "SG.Mgmt", StepDependency: types.StepDependency{ResourceGroup: "mgmt-rg", Step: "deploy"}}) + assert.True(t, ok, "stamped step") + }, + }, + { + name: "validation steps keyed per stamp", + stampPipelines: func() map[Stamp]map[string]*types.Pipeline { + valStep := &types.GenericValidationStep{StepMeta: types.StepMeta{Name: "validate"}} + return map[Stamp]map[string]*types.Pipeline{ + mustStamp("1"): { + "SG.Infra": infraPipeline, + "SG.Mgmt": makePipelineWithValidation("SG.Mgmt", + &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-1", Subscription: "sub-1"}, + []types.ValidationStep{valStep}, deploy), + }, + mustStamp("2"): { + "SG.Infra": infraPipeline, + "SG.Mgmt": makePipelineWithValidation("SG.Mgmt", + &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-2", Subscription: "sub-2"}, + []types.ValidationStep{valStep}, deploy), + }, + } + }(), + validate: func(t *testing.T, result *Graph) { + for _, stamp := range []Stamp{mustStamp("1"), mustStamp("2")} { + key := Identifier{Stamp: stamp, ServiceGroup: "SG.Mgmt", StepDependency: types.StepDependency{ResourceGroup: "mgmt-rg", Step: "validate"}} + assert.Contains(t, result.ServiceValidationSteps, key) + } + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + buildAndValidate(t, topo, testCase.stampPipelines, testCase.validate) + }) + } +} + +// TestStampedMixedSiblings tests stamped graph construction with: +// +// SG.Regional (unstamped) → SG.Svc (unstamped) + SG.Mgmt (stamped) +func TestStampedMixedSiblings(t *testing.T) { + deploy := &types.ShellStep{StepMeta: types.StepMeta{Name: "deploy"}} + + topo := makeTopology(topology.Service{ + ServiceGroup: "SG.Regional", Purpose: "regional", PipelinePath: "regional.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.Svc", Purpose: "svc", PipelinePath: "svc.yaml"}, + {ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml"}, + }, + }) + + regionalPipeline := makePipeline("SG.Regional", "regional-rg", deploy) + svcPipeline := makePipeline("SG.Svc", "svc-rg", deploy) + + stampPipelines := map[Stamp]map[string]*types.Pipeline{ + mustStamp("1"): { + "SG.Regional": regionalPipeline, + "SG.Svc": svcPipeline, + "SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-1", Subscription: "sub-1"}, deploy), + }, + mustStamp("2"): { + "SG.Regional": regionalPipeline, + "SG.Svc": svcPipeline, + "SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-2", Subscription: "sub-2"}, deploy), + }, + } + + testCases := []struct { + name string + validate func(t *testing.T, result *Graph) + }{ + { + name: "unstamped services appear once, stamped per stamp", + validate: func(t *testing.T, result *Graph) { + assert.Equal(t, 1, len(nodesForSG(result, "SG.Regional"))) + assert.Equal(t, 1, len(nodesForSG(result, "SG.Svc"))) + assert.Equal(t, 2, len(nodesForSG(result, "SG.Mgmt"))) + }, + }, + { + name: "regional parent fans out to stamped mgmt and links to unstamped svc", + validate: func(t *testing.T, result *Graph) { + regionalNode := nodesForSG(result, "SG.Regional")[0] + assert.False(t, regionalNode.Stamp.IsSet()) + + var svcChildren, mgmtChildren []Identifier + for _, child := range regionalNode.Children { + switch child.ServiceGroup { + case "SG.Svc": + svcChildren = append(svcChildren, child) + case "SG.Mgmt": + mgmtChildren = append(mgmtChildren, child) + } + } + + assert.Equal(t, 1, len(svcChildren)) + assert.False(t, svcChildren[0].Stamp.IsSet()) + + assert.Equal(t, 2, len(mgmtChildren)) + var mgmtChildStamps []string + for _, child := range mgmtChildren { + mgmtChildStamps = append(mgmtChildStamps, child.Stamp.String()) + } + slices.Sort(mgmtChildStamps) + assert.Equal(t, []string{"1", "2"}, mgmtChildStamps) + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + buildAndValidate(t, topo, stampPipelines, testCase.validate) + }) + } +} + +// TestStampedNestedChildren tests stamped graph construction with: +// +// SG.Regional (unstamped) → SG.Mgmt (stamped) → SG.MgmtDB (stamped) + SG.MgmtNet (stamped) +func TestStampedNestedChildren(t *testing.T) { + deploy := &types.ShellStep{StepMeta: types.StepMeta{Name: "deploy"}} + + topo := makeTopology(topology.Service{ + ServiceGroup: "SG.Regional", Purpose: "regional", PipelinePath: "regional.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.MgmtDB", Purpose: "db", PipelinePath: "db.yaml"}, + {ServiceGroup: "SG.MgmtNet", Purpose: "net", PipelinePath: "net.yaml"}, + }, + }, + }, + }) + + regionalPipeline := makePipeline("SG.Regional", "regional-rg", deploy) + + stampPipelines := map[Stamp]map[string]*types.Pipeline{ + mustStamp("1"): { + "SG.Regional": regionalPipeline, + "SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-1", Subscription: "sub-1"}, deploy), + "SG.MgmtDB": makePipeline("SG.MgmtDB", "db-rg", deploy), + "SG.MgmtNet": makePipeline("SG.MgmtNet", "net-rg", deploy), + }, + mustStamp("2"): { + "SG.Regional": regionalPipeline, + "SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-2", Subscription: "sub-2"}, deploy), + "SG.MgmtDB": makePipeline("SG.MgmtDB", "db-rg", deploy), + "SG.MgmtNet": makePipeline("SG.MgmtNet", "net-rg", deploy), + }, + } + + testCases := []struct { + name string + validate func(t *testing.T, result *Graph) + }{ + { + name: "node counts match topology shape", + validate: func(t *testing.T, result *Graph) { + assert.Equal(t, 1, len(nodesForSG(result, "SG.Regional"))) + assert.Equal(t, 2, len(nodesForSG(result, "SG.Mgmt"))) + assert.Equal(t, 2, len(nodesForSG(result, "SG.MgmtDB"))) + assert.Equal(t, 2, len(nodesForSG(result, "SG.MgmtNet"))) + }, + }, + { + name: "stamped children wired to same stamp as parent", + validate: func(t *testing.T, result *Graph) { + for _, mgmtNode := range nodesForSG(result, "SG.Mgmt") { + for _, child := range mgmtNode.Children { + assert.Equal(t, mgmtNode.Stamp, child.Stamp, + "parent stamp=%s child %s stamp=%s", mgmtNode.Stamp, child.ServiceGroup, child.Stamp) + } + } + }, + }, + { + name: "stamped children reference same-stamp parent", + validate: func(t *testing.T, result *Graph) { + for _, dbNode := range nodesForSG(result, "SG.MgmtDB") { + assert.True(t, dbNode.Stamp.IsSet()) + for _, parent := range dbNode.Parents { + assert.Equal(t, dbNode.Stamp, parent.Stamp) + } + } + }, + }, + { + name: "no cross-stamp wiring anywhere in graph", + validate: func(t *testing.T, result *Graph) { + for _, node := range result.Nodes { + if !node.Stamp.IsSet() { + continue + } + for _, child := range node.Children { + if child.Stamp.IsSet() { + assert.Equal(t, node.Stamp, child.Stamp, + "cross-stamp child: %s(stamp=%s) → %s(stamp=%s)", + node.ServiceGroup, node.Stamp, child.ServiceGroup, child.Stamp) + } + } + for _, parent := range node.Parents { + if parent.Stamp.IsSet() { + assert.Equal(t, node.Stamp, parent.Stamp, + "cross-stamp parent: %s(stamp=%s) ← %s(stamp=%s)", + node.ServiceGroup, node.Stamp, parent.ServiceGroup, parent.Stamp) + } + } + } + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + buildAndValidate(t, topo, stampPipelines, testCase.validate) + }) + } +} + +// TestStampedExternalDeps tests external dependency resolution across stamp boundaries. +func TestStampedExternalDeps(t *testing.T) { + deploy := &types.ShellStep{StepMeta: types.StepMeta{Name: "deploy"}} + + testCases := []struct { + name string + topo *topology.Topology + stampPipelines map[Stamp]map[string]*types.Pipeline + expectErr string + validate func(t *testing.T, result *Graph) + }{ + { + name: "stamped to stamped resolves same stamp", + topo: makeTopology(topology.Service{ + ServiceGroup: "SG.Regional", Purpose: "regional", PipelinePath: "regional.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.MgmtDB", Purpose: "db", PipelinePath: "db.yaml"}, + {ServiceGroup: "SG.MgmtNet", Purpose: "net", PipelinePath: "net.yaml"}, + }, + }, + }, + }), + stampPipelines: func() map[Stamp]map[string]*types.Pipeline { + netDeploy := &types.ShellStep{StepMeta: types.StepMeta{ + Name: "deploy", + ExternalDependsOn: []types.ExternalStepDependency{ + {ServiceGroup: "SG.MgmtDB", StepDependency: types.StepDependency{ResourceGroup: "db-rg", Step: "deploy"}}, + }, + }} + regionalPipeline := makePipeline("SG.Regional", "regional-rg", deploy) + return map[Stamp]map[string]*types.Pipeline{ + mustStamp("1"): { + "SG.Regional": regionalPipeline, + "SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-1", Subscription: "sub-1"}, deploy), + "SG.MgmtDB": makePipeline("SG.MgmtDB", "db-rg", deploy), + "SG.MgmtNet": makePipeline("SG.MgmtNet", "net-rg", netDeploy), + }, + mustStamp("2"): { + "SG.Regional": regionalPipeline, + "SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-2", Subscription: "sub-2"}, deploy), + "SG.MgmtDB": makePipeline("SG.MgmtDB", "db-rg", deploy), + "SG.MgmtNet": makePipeline("SG.MgmtNet", "net-rg", netDeploy), + }, + } + }(), + validate: func(t *testing.T, result *Graph) { + for _, netNode := range nodesForSG(result, "SG.MgmtNet") { + var dbParents []Identifier + for _, parent := range netNode.Parents { + if parent.ServiceGroup == "SG.MgmtDB" { + dbParents = append(dbParents, parent) + } + } + assert.Equal(t, 1, len(dbParents), "stamp %s net should have one db parent", netNode.Stamp) + assert.Equal(t, netNode.Stamp, dbParents[0].Stamp, "external dep resolves to same stamp") + } + }, + }, + { + name: "stamped to unstamped resolves without stamp", + topo: makeTopology(topology.Service{ + ServiceGroup: "SG.Regional", Purpose: "regional", PipelinePath: "regional.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.Svc", Purpose: "svc", PipelinePath: "svc.yaml"}, + {ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml"}, + }, + }), + stampPipelines: func() map[Stamp]map[string]*types.Pipeline { + mgmtDeploy := func(rgName string) *types.Pipeline { + return makePipelineWithRGMeta("SG.Mgmt", + &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: rgName, Subscription: "sub-" + rgName}, + &types.ShellStep{StepMeta: types.StepMeta{ + Name: "deploy", + ExternalDependsOn: []types.ExternalStepDependency{ + {ServiceGroup: "SG.Svc", StepDependency: types.StepDependency{ResourceGroup: "svc-rg", Step: "deploy"}}, + }, + }}) + } + regionalPipeline := makePipeline("SG.Regional", "regional-rg", deploy) + svcPipeline := makePipeline("SG.Svc", "svc-rg", deploy) + return map[Stamp]map[string]*types.Pipeline{ + mustStamp("1"): {"SG.Regional": regionalPipeline, "SG.Svc": svcPipeline, "SG.Mgmt": mgmtDeploy("mgmt-rg-1")}, + mustStamp("2"): {"SG.Regional": regionalPipeline, "SG.Svc": svcPipeline, "SG.Mgmt": mgmtDeploy("mgmt-rg-2")}, + } + }(), + validate: func(t *testing.T, result *Graph) { + for _, mgmtNode := range nodesForSG(result, "SG.Mgmt") { + var svcParents []Identifier + for _, parent := range mgmtNode.Parents { + if parent.ServiceGroup == "SG.Svc" { + svcParents = append(svcParents, parent) + } + } + assert.Equal(t, 1, len(svcParents), "stamp %s mgmt has one svc parent", mgmtNode.Stamp) + assert.False(t, svcParents[0].Stamp.IsSet(), "external dep to unstamped service has no stamp") + } + + svcNode := nodesForSG(result, "SG.Svc")[0] + var mgmtChildren []Identifier + for _, child := range svcNode.Children { + if child.ServiceGroup == "SG.Mgmt" { + mgmtChildren = append(mgmtChildren, child) + } + } + assert.Equal(t, 2, len(mgmtChildren), "unstamped svc has both stamped mgmt as children via external dep") + }, + }, + { + name: "unstamped to stamped rejected with expansion", + topo: makeTopology(topology.Service{ + ServiceGroup: "SG.Regional", Purpose: "regional", PipelinePath: "regional.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.Svc", Purpose: "svc", PipelinePath: "svc.yaml"}, + {ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml"}, + }, + }), + stampPipelines: func() map[Stamp]map[string]*types.Pipeline { + svcDeploy := &types.ShellStep{StepMeta: types.StepMeta{ + Name: "deploy", + ExternalDependsOn: []types.ExternalStepDependency{ + {ServiceGroup: "SG.Mgmt", StepDependency: types.StepDependency{ResourceGroup: "mgmt-rg", Step: "deploy"}}, + }, + }} + regionalPipeline := makePipeline("SG.Regional", "regional-rg", deploy) + svcPipeline := makePipeline("SG.Svc", "svc-rg", svcDeploy) + return map[Stamp]map[string]*types.Pipeline{ + mustStamp("1"): { + "SG.Regional": regionalPipeline, + "SG.Svc": svcPipeline, + "SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-1", Subscription: "sub-1"}, deploy), + }, + mustStamp("2"): { + "SG.Regional": regionalPipeline, + "SG.Svc": svcPipeline, + "SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-2", Subscription: "sub-2"}, deploy), + }, + } + }(), + expectErr: "cannot depend on stamped service", + }, + { + name: "unstamped to stamped rejected without expansion", + topo: makeTopology(topology.Service{ + ServiceGroup: "SG.Regional", Purpose: "regional", PipelinePath: "regional.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.Svc", Purpose: "svc", PipelinePath: "svc.yaml"}, + {ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml"}, + }, + }), + stampPipelines: func() map[Stamp]map[string]*types.Pipeline { + svcDeploy := &types.ShellStep{StepMeta: types.StepMeta{ + Name: "deploy", + ExternalDependsOn: []types.ExternalStepDependency{ + {ServiceGroup: "SG.Mgmt", StepDependency: types.StepDependency{ResourceGroup: "mgmt-rg", Step: "deploy"}}, + }, + }} + return map[Stamp]map[string]*types.Pipeline{ + Unstamped(): { + "SG.Regional": makePipeline("SG.Regional", "regional-rg", deploy), + "SG.Svc": makePipeline("SG.Svc", "svc-rg", svcDeploy), + "SG.Mgmt": makePipeline("SG.Mgmt", "mgmt-rg", deploy), + }, + } + }(), + expectErr: "cannot depend on stamped service", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + entrypoint := &testCase.topo.Entrypoints[0] + result, err := ForStampedEntrypoints(testCase.topo, []*topology.Entrypoint{entrypoint}, testCase.stampPipelines) + if testCase.expectErr != "" { + assert.Error(t, err) + assert.Contains(t, err.Error(), testCase.expectErr) + return + } + assert.NoError(t, err) + testCase.validate(t, result) + }) + } +} + +// TestDetectCyclesStamped verifies that cycle detection resolves child nodes by full +// Identifier (including Stamp) rather than only ServiceGroup/ResourceGroup/Step. +// +// Both stamps have the same cycle (A→B→A). If traverse matched nodes by only +// SG/RG/Step it would follow cross-stamp edges (A1→B2→A2→…) instead of the +// correct same-stamp path (A1→B1→A1), producing a misleading error. +func TestDetectCyclesStamped(t *testing.T) { + idA1 := Identifier{Stamp: mustStamp("1"), ServiceGroup: "SG.A", StepDependency: types.StepDependency{ResourceGroup: "rg", Step: "deploy"}} + idB1 := Identifier{Stamp: mustStamp("1"), ServiceGroup: "SG.B", StepDependency: types.StepDependency{ResourceGroup: "rg", Step: "deploy"}} + idA2 := Identifier{Stamp: mustStamp("2"), ServiceGroup: "SG.A", StepDependency: types.StepDependency{ResourceGroup: "rg", Step: "deploy"}} + idB2 := Identifier{Stamp: mustStamp("2"), ServiceGroup: "SG.B", StepDependency: types.StepDependency{ResourceGroup: "rg", Step: "deploy"}} + + graph := &Graph{ + Nodes: []Node{ + {Identifier: idA1, Children: []Identifier{idB1}}, + {Identifier: idB1, Children: []Identifier{idA1}}, + {Identifier: idA2, Children: []Identifier{idB2}}, + {Identifier: idB2, Children: []Identifier{idA2}}, + }, + } + + err := graph.detectCycles() + assert.Error(t, err) + assert.Contains(t, err.Error(), "cycle detected") + + msg := err.Error() + hasStamp1 := strings.Contains(msg, "stamp=1") + hasStamp2 := strings.Contains(msg, "stamp=2") + assert.True(t, hasStamp1 || hasStamp2, "cycle path should include stamp info") + assert.False(t, hasStamp1 && hasStamp2, "cycle path should stay within one stamp, got: %s", msg) +} + +// TestNoExpansionMode verifies that stamped services appear once with an empty stamp +// when no non-empty stamp keys are provided. +func TestNoExpansionMode(t *testing.T) { + deploy := &types.ShellStep{StepMeta: types.StepMeta{Name: "deploy"}} + + testCases := []struct { + name string + topo *topology.Topology + pipelines map[string]*types.Pipeline + validate func(t *testing.T, result *Graph) + }{ + { + name: "ForEntrypoint convenience wrapper", + topo: makeTopology(topology.Service{ + ServiceGroup: "SG.Regional", Purpose: "regional", PipelinePath: "regional.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.Svc", Purpose: "svc", PipelinePath: "svc.yaml"}, + {ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml"}, + }, + }), + pipelines: map[string]*types.Pipeline{ + "SG.Regional": makePipeline("SG.Regional", "regional-rg", deploy), + "SG.Svc": makePipeline("SG.Svc", "svc-rg", deploy), + "SG.Mgmt": makePipeline("SG.Mgmt", "mgmt-rg", deploy), + }, + }, + { + name: "ForEntrypoints with flat pipelines", + topo: makeTopology(topology.Service{ + ServiceGroup: "SG.Infra", Purpose: "infra", PipelinePath: "infra.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml"}, + }, + }), + pipelines: map[string]*types.Pipeline{ + "SG.Infra": makePipeline("SG.Infra", "infra-rg", deploy), + "SG.Mgmt": makePipeline("SG.Mgmt", "mgmt-rg", deploy), + }, + }, + { + name: "stamped to stamped external dep without expansion", + topo: makeTopology(topology.Service{ + ServiceGroup: "SG.Regional", Purpose: "regional", PipelinePath: "regional.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.MgmtDB", Purpose: "db", PipelinePath: "db.yaml"}, + {ServiceGroup: "SG.MgmtNet", Purpose: "net", PipelinePath: "net.yaml"}, + }, + }, + }, + }), + pipelines: func() map[string]*types.Pipeline { + netDeploy := &types.ShellStep{StepMeta: types.StepMeta{ + Name: "deploy", + ExternalDependsOn: []types.ExternalStepDependency{ + {ServiceGroup: "SG.MgmtDB", StepDependency: types.StepDependency{ResourceGroup: "db-rg", Step: "deploy"}}, + }, + }} + return map[string]*types.Pipeline{ + "SG.Regional": makePipeline("SG.Regional", "regional-rg", deploy), + "SG.Mgmt": makePipeline("SG.Mgmt", "mgmt-rg", deploy), + "SG.MgmtDB": makePipeline("SG.MgmtDB", "db-rg", deploy), + "SG.MgmtNet": makePipeline("SG.MgmtNet", "net-rg", netDeploy), + } + }(), + validate: func(t *testing.T, result *Graph) { + netNodes := nodesForSG(result, "SG.MgmtNet") + assert.Equal(t, 1, len(netNodes), "stamped service appears once in no-expansion mode") + var dbParents []Identifier + for _, parent := range netNodes[0].Parents { + if parent.ServiceGroup == "SG.MgmtDB" { + dbParents = append(dbParents, parent) + } + } + assert.Equal(t, 1, len(dbParents), "net should have one db parent via external dep") + assert.False(t, dbParents[0].Stamp.IsSet(), "external dep in no-expansion mode has no stamp") + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + entrypoint := &testCase.topo.Entrypoints[0] + result, err := ForEntrypoints(testCase.topo, []*topology.Entrypoint{entrypoint}, testCase.pipelines) + if !assert.NoError(t, err) { + return + } + + mgmtNodes := nodesForSG(result, "SG.Mgmt") + assert.Equal(t, 1, len(mgmtNodes), "stamped service appears once in no-expansion mode") + assert.False(t, mgmtNodes[0].Stamp.IsSet()) + + if testCase.validate != nil { + testCase.validate(t, result) + } + }) + } +} + +// TestForPipelineStampedService verifies that ForPipeline works with a stamped service, +// using a synthetic stamp key internally. +func TestForPipelineStampedService(t *testing.T) { + deploy := &types.ShellStep{StepMeta: types.StepMeta{Name: "deploy"}} + + service := &topology.Service{ + ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml", + } + pipeline := makePipeline("SG.Mgmt", "mgmt-rg", deploy) + + result, err := ForPipeline(service, pipeline) + assert.NoError(t, err) + assert.Equal(t, 1, len(result.Nodes)) + assert.False(t, result.Nodes[0].Stamp.IsSet()) +} + +// TestStampedEntrypointDOT verifies MarshalDOT output for all non-error stamp combinations. +// +// Topology (modeled after aro-hcp): +// +// Region (unstamped, step: deploy) +// ├─ Service.Infra (unstamped, step: deploy) +// └─ Management.Infra (stamped, steps: infra → deploy, infra → configure) +// ├─ Maestro.Agent (stamped, step: deploy, external dep on Service.Infra) - target unstamped +// └─ Fleet.Registration (stamped, step: deploy, external dep on Maestro.Agent) - both stamped +// +// Covers: +// - unstamped → unstamped topology: Region → Service.Infra +// - unstamped → stamped fan-out: Region → Management.Infra ×2 stamps +// - stamped → stamped same-stamp topology: Management.Infra → Maestro.Agent / Fleet.Registration +// - fan-in: both Management.Infra leaves (deploy + configure) → each child root +// - external dep stamped → stamped: Fleet.Registration → Maestro.Agent (same stamp) +// - external dep stamped → unstamped: Maestro.Agent → Service.Infra +func TestStampedEntrypointDOT(t *testing.T) { + deploy := &types.ShellStep{StepMeta: types.StepMeta{Name: "deploy"}} + + mgmtInfra := &types.ShellStep{StepMeta: types.StepMeta{Name: "infra"}} + mgmtDeploy := &types.ShellStep{StepMeta: types.StepMeta{ + Name: "deploy", + DependsOn: []types.StepDependency{{ResourceGroup: "mgmt-rg", Step: "infra"}}, + }} + mgmtConfigure := &types.ShellStep{StepMeta: types.StepMeta{ + Name: "configure", + DependsOn: []types.StepDependency{{ResourceGroup: "mgmt-rg", Step: "infra"}}, + }} + + maestroDeploy := &types.ShellStep{StepMeta: types.StepMeta{ + Name: "deploy", + ExternalDependsOn: []types.ExternalStepDependency{ + {ServiceGroup: "Microsoft.Azure.ARO.HCP.Service.Infra", StepDependency: types.StepDependency{ResourceGroup: "svc-rg", Step: "deploy"}}, + }, + }} + + fleetDeploy := &types.ShellStep{StepMeta: types.StepMeta{ + Name: "deploy", + ExternalDependsOn: []types.ExternalStepDependency{ + {ServiceGroup: "Microsoft.Azure.ARO.HCP.Maestro.Agent", StepDependency: types.StepDependency{ResourceGroup: "maestro-rg", Step: "deploy"}}, + }, + }} + + topo := makeTopology(topology.Service{ + ServiceGroup: "Microsoft.Azure.ARO.HCP.Region", Purpose: "region", PipelinePath: "region.yaml", + Children: []topology.Service{ + {ServiceGroup: "Microsoft.Azure.ARO.HCP.Service.Infra", Purpose: "svc infra", PipelinePath: "svc.yaml"}, + {ServiceGroup: "Microsoft.Azure.ARO.HCP.Management.Infra", Purpose: "mgmt infra", Stamped: ptr(true), PipelinePath: "mgmt.yaml", + Children: []topology.Service{ + {ServiceGroup: "Microsoft.Azure.ARO.HCP.Maestro.Agent", Purpose: "maestro agent", PipelinePath: "maestro-agent.yaml"}, + {ServiceGroup: "Microsoft.Azure.ARO.HCP.Fleet.Registration", Purpose: "fleet registration", PipelinePath: "fleet-reg.yaml"}, + }, + }, + }, + }) + + regionPipeline := makePipeline("Microsoft.Azure.ARO.HCP.Region", "region-rg", deploy) + svcPipeline := makePipeline("Microsoft.Azure.ARO.HCP.Service.Infra", "svc-rg", deploy) + + stampPipelines := map[Stamp]map[string]*types.Pipeline{ + mustStamp("1"): { + "Microsoft.Azure.ARO.HCP.Region": regionPipeline, + "Microsoft.Azure.ARO.HCP.Service.Infra": svcPipeline, + "Microsoft.Azure.ARO.HCP.Management.Infra": makePipelineWithRGMeta("Microsoft.Azure.ARO.HCP.Management.Infra", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-1", Subscription: "sub-1"}, mgmtInfra, mgmtDeploy, mgmtConfigure), + "Microsoft.Azure.ARO.HCP.Maestro.Agent": makePipeline("Microsoft.Azure.ARO.HCP.Maestro.Agent", "maestro-rg", maestroDeploy), + "Microsoft.Azure.ARO.HCP.Fleet.Registration": makePipeline("Microsoft.Azure.ARO.HCP.Fleet.Registration", "fleet-rg", fleetDeploy), + }, + mustStamp("2"): { + "Microsoft.Azure.ARO.HCP.Region": regionPipeline, + "Microsoft.Azure.ARO.HCP.Service.Infra": svcPipeline, + "Microsoft.Azure.ARO.HCP.Management.Infra": makePipelineWithRGMeta("Microsoft.Azure.ARO.HCP.Management.Infra", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-2", Subscription: "sub-2"}, mgmtInfra, mgmtDeploy, mgmtConfigure), + "Microsoft.Azure.ARO.HCP.Maestro.Agent": makePipeline("Microsoft.Azure.ARO.HCP.Maestro.Agent", "maestro-rg", maestroDeploy), + "Microsoft.Azure.ARO.HCP.Fleet.Registration": makePipeline("Microsoft.Azure.ARO.HCP.Fleet.Registration", "fleet-rg", fleetDeploy), + }, + } + + entrypoint := &topo.Entrypoints[0] + result, err := ForStampedEntrypoints(topo, []*topology.Entrypoint{entrypoint}, stampPipelines) + assert.NoError(t, err) + + encoded, err := MarshalDOT(result) + assert.NoError(t, err) + + testutil.CompareWithFixture(t, encoded, testutil.WithExtension(".dot")) +} + +func TestForStampedPipeline(t *testing.T) { + deploy := &types.ShellStep{StepMeta: types.StepMeta{Name: "deploy"}} + configure := &types.ShellStep{StepMeta: types.StepMeta{ + Name: "configure", + DependsOn: []types.StepDependency{{ResourceGroup: "mgmt-rg", Step: "deploy"}}, + }} + + testCases := []struct { + name string + service *topology.Service + stampPipelines map[Stamp]map[string]*types.Pipeline + validate func(t *testing.T, result *Graph) + }{ + { + name: "unstamped service produces unstamped nodes", + service: &topology.Service{ + ServiceGroup: "SG.Infra", Purpose: "infra", PipelinePath: "infra.yaml", + }, + stampPipelines: map[Stamp]map[string]*types.Pipeline{ + Unstamped(): {"SG.Infra": makePipeline("SG.Infra", "infra-rg", deploy)}, + }, + validate: func(t *testing.T, result *Graph) { + assert.Equal(t, 1, len(result.Nodes)) + assert.False(t, result.Nodes[0].Stamp.IsSet()) + assert.Equal(t, 1, len(result.ResourceGroups)) + _, exists := result.ResourceGroups[ResourceGroupKey{Name: "infra-rg"}] + assert.True(t, exists) + }, + }, + { + name: "stamped service expands per stamp", + service: &topology.Service{ + ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml", + }, + stampPipelines: map[Stamp]map[string]*types.Pipeline{ + mustStamp("1"): {"SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-1", Subscription: "sub-1"}, deploy)}, + mustStamp("2"): {"SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-2", Subscription: "sub-2"}, deploy)}, + }, + validate: func(t *testing.T, result *Graph) { + assert.Equal(t, 2, len(result.Nodes)) + assert.Equal(t, []string{"1", "2"}, stampSets(result.Nodes)) + assert.Equal(t, 2, len(result.ResourceGroups)) + rg1 := result.ResourceGroups[ResourceGroupKey{Stamp: mustStamp("1"), Name: "mgmt-rg"}] + rg2 := result.ResourceGroups[ResourceGroupKey{Stamp: mustStamp("2"), Name: "mgmt-rg"}] + assert.Equal(t, "mgmt-rg-1", rg1.ResourceGroup) + assert.Equal(t, "mgmt-rg-2", rg2.ResourceGroup) + assert.Equal(t, "sub-1", rg1.Subscription) + assert.Equal(t, "sub-2", rg2.Subscription) + }, + }, + { + name: "children are stripped", + service: &topology.Service{ + ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml", + Children: []topology.Service{ + {ServiceGroup: "SG.Child", Purpose: "child", PipelinePath: "child.yaml"}, + }, + }, + stampPipelines: map[Stamp]map[string]*types.Pipeline{ + mustStamp("1"): {"SG.Mgmt": makePipeline("SG.Mgmt", "mgmt-rg", deploy)}, + }, + validate: func(t *testing.T, result *Graph) { + assert.Equal(t, 1, len(result.Services)) + _, exists := result.Services["SG.Child"] + assert.False(t, exists, "children must not appear in graph") + }, + }, + { + name: "step ordering preserved within stamped pipeline", + service: &topology.Service{ + ServiceGroup: "SG.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml", + }, + stampPipelines: map[Stamp]map[string]*types.Pipeline{ + mustStamp("1"): {"SG.Mgmt": makePipeline("SG.Mgmt", "mgmt-rg", deploy, configure)}, + }, + validate: func(t *testing.T, result *Graph) { + assert.Equal(t, 2, len(result.Nodes)) + deployID := Identifier{Stamp: mustStamp("1"), ServiceGroup: "SG.Mgmt", StepDependency: types.StepDependency{ResourceGroup: "mgmt-rg", Step: "deploy"}} + configureID := Identifier{Stamp: mustStamp("1"), ServiceGroup: "SG.Mgmt", StepDependency: types.StepDependency{ResourceGroup: "mgmt-rg", Step: "configure"}} + configureNode := result.Nodes[slices.IndexFunc(result.Nodes, func(n Node) bool { return n.Identifier == configureID })] + assert.Contains(t, configureNode.Parents, deployID) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result, err := ForStampedPipeline(tc.service, tc.stampPipelines) + require.NoError(t, err) + tc.validate(t, result) + }) + } +} diff --git a/pipelines/graph/graph_test.go b/pipelines/graph/graph_test.go index 8e3aafb8..a880e328 100644 --- a/pipelines/graph/graph_test.go +++ b/pipelines/graph/graph_test.go @@ -105,6 +105,54 @@ func TestForEntrypointDuplicateResourceGroups(t *testing.T) { } } +func TestFindLeavesErrorsOnMissingChild(t *testing.T) { + parentID := Identifier{ + ServiceGroup: "svc", + StepDependency: types.StepDependency{ + ResourceGroup: "rg", + Step: "parent", + }, + } + missingChildID := Identifier{ + ServiceGroup: "svc", + StepDependency: types.StepDependency{ + ResourceGroup: "rg", + Step: "missing", + }, + } + + graphBuilder := &graphBuilder{ + Graph: &Graph{ + Services: map[string]*topology.Service{ + "svc": {ServiceGroup: "svc"}, + }, + ResourceGroups: map[ResourceGroupKey]*types.ResourceGroupMeta{ + {Name: "rg"}: {Name: "rg"}, + }, + Steps: map[Identifier]types.Step{ + parentID: &types.GenericStep{StepMeta: types.StepMeta{Name: "parent"}}, + }, + Nodes: []Node{ + { + Identifier: parentID, + Children: []Identifier{missingChildID}, + }, + }, + }, + } + + _, err := graphBuilder.findLeaves(graphBuilder.Nodes) + if err == nil { + t.Fatal("expected error for missing child lookup, got nil") + } + if !strings.Contains(err.Error(), "missing") { + t.Fatalf("expected error to mention missing child, got: %v", err) + } + if !strings.Contains(err.Error(), "parent") { + t.Fatalf("expected error to mention parent node, got: %v", err) + } +} + func loadTestdata(t *testing.T, topologyPath string) (*topology.Topology, *topology.Entrypoint, *topology.Service, map[string]*types.Pipeline) { t.Helper() diff --git a/pipelines/graph/testdata/zz_fixture_TestStampedEntrypointDOT.dot b/pipelines/graph/testdata/zz_fixture_TestStampedEntrypointDOT.dot new file mode 100644 index 00000000..c3e0e4ca --- /dev/null +++ b/pipelines/graph/testdata/zz_fixture_TestStampedEntrypointDOT.dot @@ -0,0 +1,36 @@ +digraph regexp { + fontname="Helvetica,Arial,sans-serif" + node [fontname="Helvetica,Arial,sans-serif"] + edge [fontname="Helvetica,Arial,sans-serif"] + "Region_region-rg_deploy" [label="Region/region-rg/deploy"]; + "Region_region-rg_deploy" -> "Service.Infra_svc-rg_deploy"; + "Region_region-rg_deploy" -> "Management.Infra_mgmt-rg_infra_1"; + "Region_region-rg_deploy" -> "Management.Infra_mgmt-rg_infra_2"; + "Service.Infra_svc-rg_deploy" [label="Service.Infra/svc-rg/deploy"]; + "Service.Infra_svc-rg_deploy" -> "Maestro.Agent_maestro-rg_deploy_1"; + "Service.Infra_svc-rg_deploy" -> "Maestro.Agent_maestro-rg_deploy_2"; + "Management.Infra_mgmt-rg_configure_1" [label="Management.Infra/mgmt-rg-1/configure (stamp=1)" style=filled fillcolor="#B3D9FF"]; + "Management.Infra_mgmt-rg_configure_1" -> "Maestro.Agent_maestro-rg_deploy_1"; + "Management.Infra_mgmt-rg_configure_1" -> "Fleet.Registration_fleet-rg_deploy_1"; + "Management.Infra_mgmt-rg_deploy_1" [label="Management.Infra/mgmt-rg-1/deploy (stamp=1)" style=filled fillcolor="#B3D9FF"]; + "Management.Infra_mgmt-rg_deploy_1" -> "Maestro.Agent_maestro-rg_deploy_1"; + "Management.Infra_mgmt-rg_deploy_1" -> "Fleet.Registration_fleet-rg_deploy_1"; + "Management.Infra_mgmt-rg_infra_1" [label="Management.Infra/mgmt-rg-1/infra (stamp=1)" style=filled fillcolor="#B3D9FF"]; + "Management.Infra_mgmt-rg_infra_1" -> "Management.Infra_mgmt-rg_configure_1"; + "Management.Infra_mgmt-rg_infra_1" -> "Management.Infra_mgmt-rg_deploy_1"; + "Management.Infra_mgmt-rg_configure_2" [label="Management.Infra/mgmt-rg-2/configure (stamp=2)" style=filled fillcolor="#FFD9B3"]; + "Management.Infra_mgmt-rg_configure_2" -> "Maestro.Agent_maestro-rg_deploy_2"; + "Management.Infra_mgmt-rg_configure_2" -> "Fleet.Registration_fleet-rg_deploy_2"; + "Management.Infra_mgmt-rg_deploy_2" [label="Management.Infra/mgmt-rg-2/deploy (stamp=2)" style=filled fillcolor="#FFD9B3"]; + "Management.Infra_mgmt-rg_deploy_2" -> "Maestro.Agent_maestro-rg_deploy_2"; + "Management.Infra_mgmt-rg_deploy_2" -> "Fleet.Registration_fleet-rg_deploy_2"; + "Management.Infra_mgmt-rg_infra_2" [label="Management.Infra/mgmt-rg-2/infra (stamp=2)" style=filled fillcolor="#FFD9B3"]; + "Management.Infra_mgmt-rg_infra_2" -> "Management.Infra_mgmt-rg_configure_2"; + "Management.Infra_mgmt-rg_infra_2" -> "Management.Infra_mgmt-rg_deploy_2"; + "Maestro.Agent_maestro-rg_deploy_1" [label="Maestro.Agent/maestro-rg/deploy (stamp=1)" style=filled fillcolor="#B3D9FF"]; + "Maestro.Agent_maestro-rg_deploy_1" -> "Fleet.Registration_fleet-rg_deploy_1"; + "Maestro.Agent_maestro-rg_deploy_2" [label="Maestro.Agent/maestro-rg/deploy (stamp=2)" style=filled fillcolor="#FFD9B3"]; + "Maestro.Agent_maestro-rg_deploy_2" -> "Fleet.Registration_fleet-rg_deploy_2"; + "Fleet.Registration_fleet-rg_deploy_1" [label="Fleet.Registration/fleet-rg/deploy (stamp=1)" style=filled fillcolor="#B3D9FF"]; + "Fleet.Registration_fleet-rg_deploy_2" [label="Fleet.Registration/fleet-rg/deploy (stamp=2)" style=filled fillcolor="#FFD9B3"]; +} \ No newline at end of file diff --git a/pipelines/topology/types.go b/pipelines/topology/types.go index ef37372d..558d6953 100644 --- a/pipelines/topology/types.go +++ b/pipelines/topology/types.go @@ -48,7 +48,7 @@ type Service struct { ExternalParent *string `json:"externalParent,omitempty"` // Stamped marks this service group as stamp-scoped. Children inherit stamped from their parent. - Stamped bool `json:"stamped,omitempty"` + Stamped *bool `json:"stamped,omitempty"` } // Entrypoint describes an individual pipeline in the tree. @@ -148,14 +148,22 @@ func (t *Topology) PropagateStamped() { } func propagateStamped(s *Service, parentStamped bool) { - if parentStamped { - s.Stamped = true + if parentStamped && s.Stamped == nil { + s.Stamped = ptr(true) } for i := range s.Children { - propagateStamped(&s.Children[i], s.Stamped) + propagateStamped(&s.Children[i], s.IsStamped()) } } +func (s *Service) IsStamped() bool { + return s.Stamped != nil && *s.Stamped +} + +func ptr[T any](v T) *T { + return &v +} + func (t *Topology) Validate() error { return (&validator{ seen: sets.New[string](), @@ -203,7 +211,7 @@ type validator struct { func (v *validator) validate(t *Topology) error { for _, root := range t.Services { - if err := v.walk(root); err != nil { + if err := v.walk(root, false); err != nil { return err } } @@ -234,7 +242,7 @@ func (e *InvalidServiceGroupError) Error() string { return fmt.Sprintf("invalid service group %s, must be of form Microsoft.Azure.ARO.Service.Component(.Subcomponent)?", e.ServiceGroup) } -func (v *validator) walk(s Service) error { +func (v *validator) walk(s Service, parentStamped bool) error { if !strings.HasPrefix(s.ServiceGroup, "Microsoft.Azure.ARO.") { return &InvalidServiceGroupError{ServiceGroup: s.ServiceGroup} } @@ -248,6 +256,10 @@ func (v *validator) walk(s Service) error { return &InvalidServiceGroupError{ServiceGroup: s.ServiceGroup} } + if parentStamped && s.Stamped != nil && !*s.Stamped { + return fmt.Errorf("service %s explicitly sets stamped=false but its parent is stamped; stamped subtrees cannot have opt-outs", s.ServiceGroup) + } + if v.seen.Has(s.ServiceGroup) { v.duplicates.Insert(s.ServiceGroup) } @@ -260,8 +272,10 @@ func (v *validator) walk(s Service) error { return fmt.Errorf("failed to default pipeline: %w", err) } + // parentStamped carries inherited state through intermediates with nil Stamped + effectiveStamped := parentStamped || s.IsStamped() for _, child := range s.Children { - if err := v.walk(child); err != nil { + if err := v.walk(child, effectiveStamped); err != nil { return err } } diff --git a/pipelines/topology/types_test.go b/pipelines/topology/types_test.go index 5adbbb62..ff2440fa 100644 --- a/pipelines/topology/types_test.go +++ b/pipelines/topology/types_test.go @@ -216,6 +216,52 @@ services: pipeline: foo`, err: true, }, + { + name: "explicit false under stamped parent rejected", + input: `services: +- serviceGroup: Microsoft.Azure.ARO.HCP + pipelinePath: foo + purpose: root + stamped: true + children: + - serviceGroup: Microsoft.Azure.ARO.HCP.Child + pipelinePath: bar + purpose: child + stamped: false`, + err: true, + }, + { + name: "explicit false grandchild under stamped grandparent with nil intermediate rejected", + input: `services: +- serviceGroup: Microsoft.Azure.ARO.HCP + pipelinePath: foo + purpose: root + stamped: true + children: + - serviceGroup: Microsoft.Azure.ARO.HCP.Middle + pipelinePath: bar + purpose: middle + children: + - serviceGroup: Microsoft.Azure.ARO.HCP.Middle.Leaf + pipelinePath: baz + purpose: leaf + stamped: false`, + err: true, + }, + { + name: "explicit true under stamped parent tolerated", + input: `services: +- serviceGroup: Microsoft.Azure.ARO.HCP + pipelinePath: foo + purpose: root + stamped: true + children: + - serviceGroup: Microsoft.Azure.ARO.HCP.Child + pipelinePath: bar + purpose: child + stamped: true`, + err: false, + }, } { t.Run(testCase.name, func(t *testing.T) { var topo Topology @@ -351,7 +397,7 @@ func TestPropagateStamped(t *testing.T) { Services: []Service{ { ServiceGroup: "parent", - Stamped: true, + Stamped: ptr(true), Children: []Service{ {ServiceGroup: "child-a"}, {ServiceGroup: "child-b", Children: []Service{ @@ -365,11 +411,11 @@ func TestPropagateStamped(t *testing.T) { Services: []Service{ { ServiceGroup: "parent", - Stamped: true, + Stamped: ptr(true), Children: []Service{ - {ServiceGroup: "child-a", Stamped: true}, - {ServiceGroup: "child-b", Stamped: true, Children: []Service{ - {ServiceGroup: "grandchild", Stamped: true}, + {ServiceGroup: "child-a", Stamped: ptr(true)}, + {ServiceGroup: "child-b", Stamped: ptr(true), Children: []Service{ + {ServiceGroup: "grandchild", Stamped: ptr(true)}, }}, }, }, @@ -382,7 +428,7 @@ func TestPropagateStamped(t *testing.T) { Services: []Service{ { ServiceGroup: "stamped-parent", - Stamped: true, + Stamped: ptr(true), Children: []Service{ {ServiceGroup: "stamped-child"}, }, @@ -399,9 +445,9 @@ func TestPropagateStamped(t *testing.T) { Services: []Service{ { ServiceGroup: "stamped-parent", - Stamped: true, + Stamped: ptr(true), Children: []Service{ - {ServiceGroup: "stamped-child", Stamped: true}, + {ServiceGroup: "stamped-child", Stamped: ptr(true)}, }, }, { @@ -413,6 +459,33 @@ func TestPropagateStamped(t *testing.T) { }, }, }, + { + name: "child with explicit stamped=true is preserved", + input: Topology{ + Services: []Service{ + { + ServiceGroup: "parent", + Stamped: ptr(true), + Children: []Service{ + {ServiceGroup: "child-explicit", Stamped: ptr(true)}, + {ServiceGroup: "child-nil"}, + }, + }, + }, + }, + expected: Topology{ + Services: []Service{ + { + ServiceGroup: "parent", + Stamped: ptr(true), + Children: []Service{ + {ServiceGroup: "child-explicit", Stamped: ptr(true)}, + {ServiceGroup: "child-nil", Stamped: ptr(true)}, + }, + }, + }, + }, + }, { name: "no stamped services, no changes", input: Topology{ @@ -465,11 +538,11 @@ func TestLoad_PropagateStamped(t *testing.T) { Services: []Service{ { ServiceGroup: "parent", - Stamped: true, + Stamped: ptr(true), Children: []Service{ - {ServiceGroup: "child-a", Stamped: true}, - {ServiceGroup: "child-b", Stamped: true, Children: []Service{ - {ServiceGroup: "grandchild", Stamped: true}, + {ServiceGroup: "child-a", Stamped: ptr(true)}, + {ServiceGroup: "child-b", Stamped: ptr(true), Children: []Service{ + {ServiceGroup: "grandchild", Stamped: ptr(true)}, }}, }, }, @@ -726,20 +799,20 @@ func TestLoadCombined(t *testing.T) { ServiceGroup: "Microsoft.Azure.ARO.HCP", PipelinePath: "foo", Purpose: "root", - Stamped: true, + Stamped: ptr(true), Children: []Service{ { ServiceGroup: "Microsoft.Azure.ARO.HCP.Child", PipelinePath: "bar", Purpose: "grafted child", - Stamped: true, + Stamped: ptr(true), ExternalParent: func() *string { s := "Microsoft.Azure.ARO.HCP"; return &s }(), Children: []Service{ { ServiceGroup: "Microsoft.Azure.ARO.HCP.Child.Grand", PipelinePath: "baz", Purpose: "grandchild", - Stamped: true, + Stamped: ptr(true), }, }, },