From a4a1d770db4d8a954162d68773addd874c60c546 Mon Sep 17 00:00:00 2001 From: Gerd Oberlechner Date: Thu, 18 Jun 2026 23:14:28 +0200 Subject: [PATCH 1/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. No post-hoc merge pass needed. Key changes: - ForEntrypoints accepts stamp-keyed pipelines (map[string]map[string]*Pipeline). Non-stamped callers use ForEntrypoint which wraps pipelines as {"": pipelines}. ForStampedEntrypoint is removed — callers pass stamp pipelines directly. - 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 empty stamp. - StepKey removed — Identifier used as map key for Steps. - GetResourceGroup narrowed to accept ResourceGroupKey instead of Identifier. - ConfigResolver.GetRegionStampConfiguration resolves config with stamp dimension. - Service.Stamped changed to *bool — explicit false preserved and validated, distinguishing "unset" from "intentionally not stamped". Breaking changes on Graph, ForEntrypoints, and ForStampedEntrypoint (removed). --- config/config.go | 33 +- config/config_internal_test.go | 6 +- config/config_test.go | 58 +- pipelines/graph/graph.go | 402 ++++++---- pipelines/graph/graph_stamped_test.go | 704 ++++++++++++++++++ pipelines/graph/graph_test.go | 8 +- .../testdata/zz_fixture_TestForEntrypoint.dot | 98 +-- .../testdata/zz_fixture_TestForPipeline.dot | 24 +- .../zz_fixture_TestStampedEntrypointDOT.dot | 20 + pipelines/topology/types.go | 28 +- pipelines/topology/types_test.go | 103 ++- pipelines/types/pipeline_test.go | 4 +- 12 files changed, 1259 insertions(+), 229 deletions(-) create mode 100644 pipelines/graph/graph_stamped_test.go create mode 100644 pipelines/graph/testdata/zz_fixture_TestStampedEntrypointDOT.dot diff --git a/config/config.go b/config/config.go index 36ea5e67..7b826c1b 100644 --- a/config/config.go +++ b/config/config.go @@ -81,7 +81,8 @@ type ConfigResolver interface { // GetRegions divulges the regions for which overrides are registered. GetRegions() ([]string, error) // GetRegionConfiguration resolves the configuration for a region in the cloud and environment. - GetRegionConfiguration(region string) (types.Configuration, error) + // It re-processes the configuration template with the given region and stamp. + GetRegionConfiguration(region, stamp string) (types.Configuration, error) // GetRegionOverrides fetches the overrides specific to a region, if any exist. GetRegionOverrides(region string) (types.Configuration, error) // ValueProvenance divulges how the value at 'path' is overridden to arrive at the result. @@ -204,6 +205,8 @@ func (cp *configProvider) GetResolver(configReplacements *ConfigReplacements) (C environment: configReplacements.EnvironmentReplacement, cfg: currentVariableOverrides, absoluteSchemaPath: cp.absoluteSchemaPath, + provider: cp, + replacements: *configReplacements, }, nil } @@ -211,6 +214,9 @@ type configResolver struct { cloud, environment string cfg configurationOverrides absoluteSchemaPath string + + provider *configProvider + replacements ConfigReplacements } func (cr *configResolver) ValidateSchema(config types.Configuration) error { @@ -259,10 +265,24 @@ func (cr *configResolver) GetRegions() ([]string, error) { return regions, nil } -// GetRegionConfiguration merges values to resolve the configuration for a region. -func (cr *configResolver) GetRegionConfiguration(region string) (types.Configuration, error) { - cfg := cr.cfg.Defaults - cloudCfg, hasCloud := cr.cfg.Overrides[cr.cloud] +// GetRegionConfiguration re-templates the configuration with the given region and stamp, +// then merges values to resolve the configuration for the region. +func (cr *configResolver) GetRegionConfiguration(region, stamp string) (types.Configuration, error) { + replacements := cr.replacements + replacements.RegionReplacement = region + replacements.StampReplacement = stamp + rawContent, err := PreprocessContent(cr.provider.raw, replacements.AsMap()) + if err != nil { + return nil, fmt.Errorf("failed to preprocess config for region %s stamp %s: %w", region, stamp, err) + } + + regionalOverrides := configurationOverrides{} + if err := yaml.Unmarshal(rawContent, ®ionalOverrides); err != nil { + return nil, fmt.Errorf("failed to unmarshal config for region %s stamp %s: %w", region, stamp, err) + } + + cfg := regionalOverrides.Defaults + cloudCfg, hasCloud := regionalOverrides.Overrides[cr.cloud] if !hasCloud { return nil, fmt.Errorf("the cloud %s is not found in the config", cr.cloud) } @@ -274,7 +294,6 @@ func (cr *configResolver) GetRegionConfiguration(region string) (types.Configura cfg = types.MergeConfiguration(cfg, envCfg.Defaults) regionCfg, hasRegion := envCfg.Overrides[region] if !hasRegion { - // a missing region just means we use default values regionCfg = types.Configuration{} } cfg = types.MergeConfiguration(cfg, regionCfg) @@ -350,7 +369,7 @@ func (cr *configResolver) ValueProvenance(region, path string) (*Provenance, err regionCfg = types.Configuration{} } - mergedCfg, err := cr.GetRegionConfiguration(region) + mergedCfg, err := cr.GetRegionConfiguration(region, cr.replacements.StampReplacement) if err != nil { return nil, err } diff --git a/config/config_internal_test.go b/config/config_internal_test.go index e77c112d..3f974447 100644 --- a/config/config_internal_test.go +++ b/config/config_internal_test.go @@ -33,7 +33,7 @@ func TestValidateSchema(t *testing.T) { }) require.NoError(t, err) - cfg, err := resolver.GetRegionConfiguration("uksouth") + cfg, err := resolver.GetRegionConfiguration("uksouth", "1") require.NoError(t, err) validationErr := resolver.ValidateSchema(cfg) @@ -55,7 +55,7 @@ func TestValidateSchemaWithCEL(t *testing.T) { }) require.NoError(t, err) - cfg, err := resolver.GetRegionConfiguration("uksouth") + cfg, err := resolver.GetRegionConfiguration("uksouth", "1") require.NoError(t, err) validationErr := resolver.ValidateSchema(cfg) @@ -75,7 +75,7 @@ func TestValidateSchemaWithCEL(t *testing.T) { }) require.NoError(t, err) - cfg, err := resolver.GetRegionConfiguration("uksouth") + cfg, err := resolver.GetRegionConfiguration("uksouth", "1") require.NoError(t, err) validationErr := resolver.ValidateSchema(cfg) diff --git a/config/config_test.go b/config/config_test.go index eb64e5df..e24dbb74 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -49,13 +49,69 @@ func TestConfigProvider(t *testing.T) { }) require.NoError(t, err) - cfg, err := configResolver.GetRegionConfiguration(region) + cfg, err := configResolver.GetRegionConfiguration(region, stamp) require.NoError(t, err) require.NotNil(t, cfg) testutil.CompareWithFixture(t, cfg) } +func TestGetRegionStampConfiguration(t *testing.T) { + region := "uksouth" + regionShort := "uks" + cloud := "public" + environment := "int" + + ev2, err := ev2config.ResolveConfig(cloud, region) + require.NoError(t, err) + + configProvider, err := config.NewConfigProvider("./testdata/pipelines/config.yaml") + require.NoError(t, err) + configResolver, err := configProvider.GetResolver(&config.ConfigReplacements{ + RegionReplacement: region, + RegionShortReplacement: regionShort, + StampReplacement: "1", + CloudReplacement: cloud, + EnvironmentReplacement: environment, + Ev2Config: ev2, + }) + require.NoError(t, err) + + testCases := []struct { + name string + stamp string + expectedMgmtRG string + }{ + { + name: "stamp 1", + stamp: "1", + expectedMgmtRG: "hcp-underlay-uks-mgmt-1", + }, + { + name: "stamp 2", + stamp: "2", + expectedMgmtRG: "hcp-underlay-uks-mgmt-2", + }, + { + name: "stamp 3", + stamp: "3", + expectedMgmtRG: "hcp-underlay-uks-mgmt-3", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cfg, err := configResolver.GetRegionConfiguration(region, tc.stamp) + require.NoError(t, err) + require.NotNil(t, cfg) + + mgmtRG, ok := cfg["managementClusterRG"] + require.True(t, ok, "managementClusterRG should be present in config") + require.Equal(t, tc.expectedMgmtRG, mgmtRG) + }) + } +} + func TestConfigProvenance(t *testing.T) { region := "uksouth" regionShort := "uks" diff --git a/pipelines/graph/graph.go b/pipelines/graph/graph.go index 7031dc81..4c40e8c5 100644 --- a/pipelines/graph/graph.go +++ b/pipelines/graph/graph.go @@ -15,14 +15,24 @@ import ( "github.com/Azure/ARO-Tools/pipelines/types" ) -// Dependency records a dependency on a step in a particular service group and resource group. +// 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 string 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 != "" { + 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) } @@ -39,23 +49,40 @@ type Node struct { Parents []Identifier } +// ResourceGroupKey identifies a resource group within a graph. Stamp is empty 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 string + Name string +} + +func newGraph() *Graph { + return &Graph{ + Services: map[string]*topology.Service{}, + ResourceGroups: map[ResourceGroupKey]*types.ResourceGroupMeta{}, + resourceGroupOwners: map[string]sets.Set[string]{}, + Steps: map[Identifier]types.Step{}, + Nodes: []Node{}, + ServiceValidationSteps: map[Identifier]types.ValidationStep{}, + } +} + // 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 { // 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 an empty Stamp value. + 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 @@ -67,6 +94,18 @@ type Graph struct { resourceGroupOwners map[string]sets.Set[string] } +// GetResourceGroup returns the resource group metadata for the given key. +func (c *Graph) GetResourceGroup(key ResourceGroupKey) (*types.ResourceGroupMeta, bool) { + rg, exists := c.ResourceGroups[key] + return rg, exists +} + +// 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. type Subscription struct { // ServiceGroup records the service which requested the subscription provisioning. In a multi-service execution graph, there may @@ -95,19 +134,15 @@ 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: service.Stamped, + Stamped: nil, // one pipeline in, nothing to stamp-multiply } - 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{}, - } + graph := newGraph() - if err := graph.accumulate(withoutChildren, map[string]*types.Pipeline{pipeline.ServiceGroup: pipeline}); err != nil { + stampPipelines := map[string]map[string]*types.Pipeline{ + "": {pipeline.ServiceGroup: pipeline}, + } + if err := graph.accumulate(withoutChildren, stampPipelines); err != nil { return nil, err } @@ -116,11 +151,13 @@ func ForPipeline(service *topology.Service, pipeline *types.Pipeline) (*Graph, e // ForEntrypoint generates a graph for all pipelines in the sub-tree of the topology identified by the entrypoint. func ForEntrypoint(topo *topology.Topology, entrypoint *topology.Entrypoint, pipelines map[string]*types.Pipeline) (*Graph, error) { - return ForEntrypoints(topo, []*topology.Entrypoint{entrypoint}, pipelines) + return ForEntrypoints(topo, []*topology.Entrypoint{entrypoint}, map[string]map[string]*types.Pipeline{"": pipelines}) } // ForEntrypoints generates a graph for all pipelines in the sub-trees of the topology identified by the entrypoints. -func ForEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, pipelines map[string]*types.Pipeline) (*Graph, error) { +// stampPipelines maps stamp identifiers to per-service-group pipelines. Callers that do not use stamps pass a single +// entry keyed by "" (empty string). When stamped services are encountered, the graph expands them once per stamp. +func ForEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, stampPipelines map[string]map[string]*types.Pipeline) (*Graph, error) { var roots []*topology.Service for _, entrypoint := range entrypoints { root, err := topo.Lookup(entrypoint.Identifier) @@ -130,17 +167,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{}, - } + graph := newGraph() for _, root := range roots { - if err := graph.accumulate(root, pipelines); err != nil { + if err := graph.accumulate(root, stampPipelines); err != nil { return nil, err } } @@ -156,81 +186,127 @@ func ForEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, return graph, graph.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 { +// accumulate recursively traverses the service and all children, building a graph of how steps in each service +// depend on each other. When a service is stamped, it expands nodes once per stamp; unstamped services are +// processed once with an empty stamp. +func (c *Graph) accumulate(service *topology.Service, stampPipelines map[string]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 - c.Steps[service.ServiceGroup] = map[string]map[string]types.Step{} - pipeline, exists := pipelines[service.ServiceGroup] - if !exists { - return fmt.Errorf("pipeline for service %s not found", service.ServiceGroup) - } - 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) + // Determine which stamp iterations to run: stamped services expand once per stamp key, + // unstamped services run once with an empty stamp. + type stampIteration struct { + stamp string + pipeline *types.Pipeline + } + var iterations []stampIteration + if service.IsStamped() { + for _, stamp := range slices.Sorted(maps.Keys(stampPipelines)) { + if stamp == "" { + continue + } + pipeline, exists := stampPipelines[stamp][service.ServiceGroup] + if !exists { + return fmt.Errorf("pipeline for service %s not found in stamp %s", service.ServiceGroup, stamp) + } + iterations = append(iterations, stampIteration{stamp: stamp, pipeline: pipeline}) + } + if len(iterations) == 0 { + return fmt.Errorf("stamped service %s requires at least one non-empty stamp key, but no non-empty stamp keys were provided", service.ServiceGroup) + } + } else { + // Unstamped services have the same pipeline regardless of stamp key — grab from any entry. + var pipeline *types.Pipeline + for _, pipelines := range stampPipelines { + if p, exists := pipelines[service.ServiceGroup]; exists { + pipeline = p + break + } + } + if pipeline == nil { + return fmt.Errorf("pipeline for service %s not found", service.ServiceGroup) + } + iterations = append(iterations, stampIteration{stamp: "", pipeline: pipeline}) } - for name, group := range resourceGroups { - if other, alreadyRecorded := c.ResourceGroups[name]; alreadyRecorded { - if !resourceGroupMetaEqual(group, other) { + + // For each stamp iteration, generate nodes and register resource groups, steps, and metadata. + // Leaf nodes are collected per stamp so that inter-service wiring connects only matching stamps. + allLeaves := map[string][]Identifier{} + for _, iter := range iterations { + resourceGroups, subscription, steps, serviceValidationSteps, nodes, err := nodesFor(iter.pipeline, iter.stamp) + if err != nil { + return fmt.Errorf("failed to generate graph for pipeline %s: %v", service.ServiceGroup, err) + } + + // Register resource groups, detecting conflicts within the same stamp and across stamp boundaries. + for name, group := range resourceGroups { + key := ResourceGroupKey{Stamp: iter.stamp, Name: name} + other, alreadyRecorded := c.ResourceGroups[key] + if alreadyRecorded && !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 + } + if alreadyRecorded { c.resourceGroupOwners[name].Insert(service.ServiceGroup) + } else { + c.ResourceGroups[key] = group + c.resourceGroupOwners[name] = sets.New(service.ServiceGroup) } - } else { - // First time recording this resource group - c.ResourceGroups[name] = group - c.resourceGroupOwners[name] = sets.New(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) + + 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) + } + c.Subscription = subscription } - c.Subscription = subscription - } - c.Steps[service.ServiceGroup] = steps - maps.Copy(c.ServiceValidationSteps, serviceValidationSteps) - c.Nodes = append(c.Nodes, nodes...) - 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) + // 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: service.ServiceGroup, StepDependency: types.StepDependency{ResourceGroup: rg, Step: stepName}}] = step + } } - if len(node.Children) == 0 { - if step.ConsideredForServiceGroupCompletion() { - leaves = append(leaves, node.Identifier) + maps.Copy(c.ServiceValidationSteps, serviceValidationSteps) + c.Nodes = append(c.Nodes, nodes...) + + // Identify leaf nodes for this stamp iteration — these will become parents of child service roots. + 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) } - } 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 len(node.Children) == 0 { + if step.ConsideredForServiceGroupCompletion() { + leaves = append(leaves, node.Identifier) } - if !childStep.ConsideredForServiceGroupCompletion() { - ignoredLeaves++ + } 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 ignoredLeaves == len(node.Children) { - leaves = append(leaves, node.Identifier) } } + allLeaves[iter.stamp] = leaves } + // Wire inter-service edges: connect this service's leaves to child service roots. for _, child := range service.Children { - if err := c.accumulate(&child, pipelines); err != nil { + if err := c.accumulate(&child, stampPipelines); err != nil { return err } @@ -242,28 +318,34 @@ func (c *Graph) accumulate(service *topology.Service, pipelines map[string]*type // 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 + // + // When stamps are involved, wiring is stamp-scoped: stamp-1 leaves connect to stamp-1 roots only. + // Unstamped leaves (stamp="") connect to all child roots that share the same stamp="" or fan out + // to all stamps when the child is stamped and the parent is not. + + for parentStamp, leaves := range allLeaves { + var roots []Identifier + // Find root nodes 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. + for i, 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 service.IsStamped() && child.IsStamped() && node.Stamp != parentStamp { + continue + } 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...) } - } - // 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 i, node := range c.Nodes { + for _, leaf := range leaves { + if node.ServiceGroup == leaf.ServiceGroup && node.ResourceGroup == leaf.ResourceGroup && node.Step == leaf.Step && node.Stamp == leaf.Stamp { + c.Nodes[i].Children = append(c.Nodes[i].Children, roots...) + } } } } @@ -277,20 +359,20 @@ func (c *Graph) lookup(node Identifier) (*topology.Service, *types.ResourceGroup if !exists { return nil, nil, nil, fmt.Errorf("service %s does not exist", node.ServiceGroup) } - resourceGroup, exists := c.ResourceGroups[node.ResourceGroup] + resourceGroup, exists := c.GetResourceGroup(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 } 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 { + if node.Identifier == id { return i, nil } } @@ -299,9 +381,9 @@ func (c *Graph) node(id Identifier) (int, error) { func (c *Graph) 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 { @@ -315,6 +397,12 @@ func (c *Graph) addExternalDependencyEdges() error { Step: dep.Step, }, } + // External dependencies don't carry stamp information. When the target service is stamped, + // resolve to the same stamp as the node declaring the dependency — stamp-1 work depends on + // stamp-1 of the target, not all stamps. Unstamped targets keep an empty stamp. + if targetService := c.Services[dep.ServiceGroup]; targetService != nil && targetService.IsStamped() { + parent.Stamp = node.Stamp + } parentNodeIdx, err := c.node(parent) if err != nil { return err @@ -327,15 +415,15 @@ func (c *Graph) 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 string) ( map[string]*types.ResourceGroupMeta, *Subscription, map[string]map[string]types.Step, @@ -343,8 +431,6 @@ 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{} @@ -367,6 +453,7 @@ func nodesFor(pipeline *types.Pipeline) ( } for _, step := range rg.ValidationSteps { serviceValidationSteps[Identifier{ + Stamp: stamp, ServiceGroup: pipeline.ServiceGroup, StepDependency: types.StepDependency{ ResourceGroup: rg.Name, @@ -376,7 +463,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 { @@ -387,6 +473,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, @@ -394,6 +481,7 @@ func nodesFor(pipeline *types.Pipeline) ( }, }, to: Identifier{ + Stamp: stamp, ServiceGroup: pipeline.ServiceGroup, StepDependency: types.StepDependency{ ResourceGroup: rg.Name, @@ -417,6 +505,7 @@ func nodesFor(pipeline *types.Pipeline) ( for stepName := range steps { node := Node{ Identifier: Identifier{ + Stamp: stamp, ServiceGroup: pipeline.ServiceGroup, StepDependency: types.StepDependency{ ResourceGroup: resourceGroup, @@ -446,6 +535,9 @@ func nodesFor(pipeline *types.Pipeline) ( } func CompareDependencies(a, b Identifier) int { + if comparison := strings.Compare(a.Stamp, b.Stamp); comparison != 0 { + return comparison + } if comparison := strings.Compare(a.ServiceGroup, b.ServiceGroup); comparison != 0 { return comparison } @@ -505,25 +597,25 @@ func (c *Graph) detectCycles() error { func traverse(node Node, all []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 + var found bool for _, candidate := range all { - if candidate.ServiceGroup == child.ServiceGroup && candidate.ResourceGroup == child.ResourceGroup && candidate.Step == child.Step { + if candidate.Identifier == child { childNode = candidate + found = true } } - if childNode.ServiceGroup == "" { - return fmt.Errorf("could not find child node %s/%s - programmer error", child.ResourceGroup, child.Step) + if !found { + return fmt.Errorf("could not find child node %s - programmer error", child) } if err := traverse(childNode, all, chain); err != nil { return err @@ -540,43 +632,51 @@ 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(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 { + stampColors := buildStampColorMap(g.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) + 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 } - // 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 +687,32 @@ func MarshalDOT(nodes []Node, serviceValidationSteps map[Identifier]types.Valida return out.Bytes(), nil } +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 buildStampColorMap(nodes []Node) map[string]string { + stamps := sets.New[string]() + for _, node := range nodes { + if node.Stamp != "" { + stamps.Insert(node.Stamp) + } + } + sorted := sets.List(stamps) + colors := make(map[string]string, len(sorted)) + for i, stamp := range sorted { + colors[stamp] = stampColorPalette[i%len(stampColorPalette)] + } + return colors +} + func shortenServiceGroup(serviceGroup string) (string, error) { parts := strings.Split(serviceGroup, ".") if len(parts) < 5 { @@ -595,3 +721,21 @@ func shortenServiceGroup(serviceGroup string) (string, error) { return strings.Join(parts[4:], "."), nil } + +func dotID(shortSG string, id Identifier) string { + if id.Stamp != "" { + 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 != "" { + 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..0dcde3b8 --- /dev/null +++ b/pipelines/graph/graph_stamped_test.go @@ -0,0 +1,704 @@ +package graph + +import ( + "slices" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "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 } + +// 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) + } + slices.Sort(stamps) + return stamps +} + +// buildAndValidate constructs a graph via ForEntrypoints and runs the validation function against it. +func buildAndValidate(t *testing.T, topo *topology.Topology, stampPipelines map[string]map[string]*types.Pipeline, validate func(t *testing.T, result *Graph)) { + t.Helper() + entrypoint := &topo.Entrypoints[0] + result, err := ForEntrypoints(topo, []*topology.Entrypoint{entrypoint}, stampPipelines) + assert.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[string]map[string]*types.Pipeline{ + "1": { + "SG.Infra": infraPipeline, + "SG.Mgmt": makePipelineWithRGMeta("SG.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-1", Subscription: "sub-1"}, deploy), + }, + "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[string]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.Empty(t, infraNodes[0].Stamp) + }, + }, + { + 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) + } + 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.NotEmpty(t, child.Stamp) + } + } + }, + }, + { + 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: "1", Name: "mgmt-rg"}] + assert.Equal(t, "mgmt-rg-1", rg1.ResourceGroup) + + rg2 := result.ResourceGroups[ResourceGroupKey{Stamp: "2", Name: "mgmt-rg"}] + assert.Equal(t, "mgmt-rg-2", rg2.ResourceGroup) + }, + }, + { + name: "GetResourceGroup resolves stamped and unstamped keys", + stampPipelines: twoStampPipelines, + validate: func(t *testing.T, result *Graph) { + rg, ok := result.GetResourceGroup(ResourceGroupKey{Name: "infra-rg"}) + assert.True(t, ok) + assert.Equal(t, "infra-rg", rg.ResourceGroup) + + rg, ok = result.GetResourceGroup(ResourceGroupKey{Stamp: "1", Name: "mgmt-rg"}) + assert.True(t, ok) + assert.Equal(t, "mgmt-rg-1", rg.ResourceGroup) + + _, ok = result.GetResourceGroup(ResourceGroupKey{Stamp: "99", Name: "mgmt-rg"}) + assert.False(t, ok) + }, + }, + { + name: "single stamp graph is valid", + stampPipelines: map[string]map[string]*types.Pipeline{ + "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.Empty(t, infraNode.Stamp) + assert.Equal(t, 1, len(infraNode.Children)) + assert.Equal(t, "1", infraNode.Children[0].Stamp) + + mgmtNode := nodesForSG(result, "SG.Mgmt")[0] + assert.Equal(t, "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: "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[string]map[string]*types.Pipeline { + valStep := &types.GenericValidationStep{StepMeta: types.StepMeta{Name: "validate"}} + return map[string]map[string]*types.Pipeline{ + "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), + }, + "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 []string{"1", "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[string]map[string]*types.Pipeline{ + "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), + }, + "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.Empty(t, regionalNode.Stamp) + + 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.Empty(t, svcChildren[0].Stamp) + + assert.Equal(t, 2, len(mgmtChildren)) + var mgmtChildStamps []string + for _, child := range mgmtChildren { + mgmtChildStamps = append(mgmtChildStamps, child.Stamp) + } + 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[string]map[string]*types.Pipeline{ + "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), + }, + "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.NotEmpty(t, dbNode.Stamp) + 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 == "" { + continue + } + for _, child := range node.Children { + if child.Stamp != "" { + 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 != "" { + 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[string]map[string]*types.Pipeline + 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[string]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[string]map[string]*types.Pipeline{ + "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), + }, + "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[string]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[string]map[string]*types.Pipeline{ + "1": {"SG.Regional": regionalPipeline, "SG.Svc": svcPipeline, "SG.Mgmt": mgmtDeploy("mgmt-rg-1")}, + "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.Empty(t, svcParents[0].Stamp, "external dep to unstamped service has empty 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") + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + buildAndValidate(t, testCase.topo, testCase.stampPipelines, testCase.validate) + }) + } +} + +// 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: "1", ServiceGroup: "SG.A", StepDependency: types.StepDependency{ResourceGroup: "rg", Step: "deploy"}} + idB1 := Identifier{Stamp: "1", ServiceGroup: "SG.B", StepDependency: types.StepDependency{ResourceGroup: "rg", Step: "deploy"}} + idA2 := Identifier{Stamp: "2", ServiceGroup: "SG.A", StepDependency: types.StepDependency{ResourceGroup: "rg", Step: "deploy"}} + idB2 := Identifier{Stamp: "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) +} + +// TestStampedServiceRequiresNonEmptyStampKey verifies that stamped services fail loudly +// when no non-empty stamp key is provided. +func TestStampedServiceRequiresNonEmptyStampKey(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"}, + }, + }) + + stampPipelines := map[string]map[string]*types.Pipeline{ + "": { + "SG.Infra": makePipeline("SG.Infra", "infra-rg", deploy), + "SG.Mgmt": makePipeline("SG.Mgmt", "mgmt-rg", deploy), + }, + } + + entrypoint := &topo.Entrypoints[0] + _, err := ForEntrypoints(topo, []*topology.Entrypoint{entrypoint}, stampPipelines) + assert.Error(t, err) + assert.Contains(t, err.Error(), "no non-empty stamp keys") +} + +// 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.Empty(t, result.Nodes[0].Stamp) +} + +// TestStampedEntrypointDOT verifies MarshalDOT produces distinct node IDs and labels for +// stamped nodes across unstamped→stamped (parent-child) and stamped→stamped (external dep) edges. +func TestStampedEntrypointDOT(t *testing.T) { + deploy := &types.ShellStep{StepMeta: types.StepMeta{Name: "deploy"}} + + netDeploy := &types.ShellStep{StepMeta: types.StepMeta{ + Name: "deploy", + ExternalDependsOn: []types.ExternalStepDependency{ + {ServiceGroup: "Microsoft.Azure.ARO.HCP.MgmtDB", StepDependency: types.StepDependency{ResourceGroup: "db-rg", Step: "deploy"}}, + }, + }} + + topo := makeTopology(topology.Service{ + ServiceGroup: "Microsoft.Azure.ARO.HCP.Infra", Purpose: "infra", PipelinePath: "infra.yaml", + Children: []topology.Service{ + {ServiceGroup: "Microsoft.Azure.ARO.HCP.Mgmt", Purpose: "mgmt", Stamped: ptr(true), PipelinePath: "mgmt.yaml", + Children: []topology.Service{ + {ServiceGroup: "Microsoft.Azure.ARO.HCP.MgmtDB", Purpose: "db", PipelinePath: "db.yaml"}, + {ServiceGroup: "Microsoft.Azure.ARO.HCP.MgmtNet", Purpose: "net", PipelinePath: "net.yaml"}, + }, + }, + }, + }) + + infraPipeline := makePipeline("Microsoft.Azure.ARO.HCP.Infra", "infra-rg", deploy) + + stampPipelines := map[string]map[string]*types.Pipeline{ + "1": { + "Microsoft.Azure.ARO.HCP.Infra": infraPipeline, + "Microsoft.Azure.ARO.HCP.Mgmt": makePipelineWithRGMeta("Microsoft.Azure.ARO.HCP.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-1", Subscription: "sub-1"}, deploy), + "Microsoft.Azure.ARO.HCP.MgmtDB": makePipeline("Microsoft.Azure.ARO.HCP.MgmtDB", "db-rg", deploy), + "Microsoft.Azure.ARO.HCP.MgmtNet": makePipeline("Microsoft.Azure.ARO.HCP.MgmtNet", "net-rg", netDeploy), + }, + "2": { + "Microsoft.Azure.ARO.HCP.Infra": infraPipeline, + "Microsoft.Azure.ARO.HCP.Mgmt": makePipelineWithRGMeta("Microsoft.Azure.ARO.HCP.Mgmt", &types.ResourceGroupMeta{Name: "mgmt-rg", ResourceGroup: "mgmt-rg-2", Subscription: "sub-2"}, deploy), + "Microsoft.Azure.ARO.HCP.MgmtDB": makePipeline("Microsoft.Azure.ARO.HCP.MgmtDB", "db-rg", deploy), + "Microsoft.Azure.ARO.HCP.MgmtNet": makePipeline("Microsoft.Azure.ARO.HCP.MgmtNet", "net-rg", netDeploy), + }, + } + + entrypoint := &topo.Entrypoints[0] + result, err := ForEntrypoints(topo, []*topology.Entrypoint{entrypoint}, stampPipelines) + assert.NoError(t, err) + + encoded, err := MarshalDOT(result) + assert.NoError(t, err) + + testutil.CompareWithFixture(t, encoded, testutil.WithExtension(".dot")) +} 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 diff --git a/pipelines/graph/testdata/zz_fixture_TestStampedEntrypointDOT.dot b/pipelines/graph/testdata/zz_fixture_TestStampedEntrypointDOT.dot new file mode 100644 index 00000000..44da400c --- /dev/null +++ b/pipelines/graph/testdata/zz_fixture_TestStampedEntrypointDOT.dot @@ -0,0 +1,20 @@ +digraph regexp { + fontname="Helvetica,Arial,sans-serif" + node [fontname="Helvetica,Arial,sans-serif"] + edge [fontname="Helvetica,Arial,sans-serif"] + "Infra_infra-rg_deploy" [label="Infra/infra-rg/deploy"]; + "Infra_infra-rg_deploy" -> "Mgmt_mgmt-rg_deploy_1"; + "Infra_infra-rg_deploy" -> "Mgmt_mgmt-rg_deploy_2"; + "Mgmt_mgmt-rg_deploy_1" [label="Mgmt/mgmt-rg-1/deploy (stamp=1)" style=filled fillcolor="#B3D9FF"]; + "Mgmt_mgmt-rg_deploy_1" -> "MgmtDB_db-rg_deploy_1"; + "Mgmt_mgmt-rg_deploy_1" -> "MgmtNet_net-rg_deploy_1"; + "Mgmt_mgmt-rg_deploy_2" [label="Mgmt/mgmt-rg-2/deploy (stamp=2)" style=filled fillcolor="#FFD9B3"]; + "Mgmt_mgmt-rg_deploy_2" -> "MgmtDB_db-rg_deploy_2"; + "Mgmt_mgmt-rg_deploy_2" -> "MgmtNet_net-rg_deploy_2"; + "MgmtDB_db-rg_deploy_1" [label="MgmtDB/db-rg/deploy (stamp=1)" style=filled fillcolor="#B3D9FF"]; + "MgmtDB_db-rg_deploy_1" -> "MgmtNet_net-rg_deploy_1"; + "MgmtDB_db-rg_deploy_2" [label="MgmtDB/db-rg/deploy (stamp=2)" style=filled fillcolor="#FFD9B3"]; + "MgmtDB_db-rg_deploy_2" -> "MgmtNet_net-rg_deploy_2"; + "MgmtNet_net-rg_deploy_1" [label="MgmtNet/net-rg/deploy (stamp=1)" style=filled fillcolor="#B3D9FF"]; + "MgmtNet_net-rg_deploy_2" [label="MgmtNet/net-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), }, }, }, diff --git a/pipelines/types/pipeline_test.go b/pipelines/types/pipeline_test.go index 057c47aa..4cdae637 100644 --- a/pipelines/types/pipeline_test.go +++ b/pipelines/types/pipeline_test.go @@ -46,7 +46,7 @@ func TestNewPipelineFromFile(t *testing.T) { }) require.NoError(t, err) - cfg, err := resolver.GetRegionConfiguration(region) + cfg, err := resolver.GetRegionConfiguration(region, stamp) assert.NoError(t, err) pipeline, err := NewPipelineFromFile("../testdata/pipeline.yaml", cfg) @@ -222,7 +222,7 @@ resourceGroups: }) require.NoError(t, err) - cfg, err := resolver.GetRegionConfiguration(region) + cfg, err := resolver.GetRegionConfiguration(region, stamp) require.NoError(t, err) _, err = NewPipelineFromBytes([]byte(tt.yaml), cfg) From cf6813cfa9bbc224f4c9e0b5d638bd9c3c1d5892 Mon Sep 17 00:00:00 2001 From: Gerd Oberlechner Date: Fri, 19 Jun 2026 13:06:21 +0200 Subject: [PATCH 2/3] refactor(graph,config): split stamp-aware APIs and extract focused functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split ForEntrypoints into ForEntrypoints (unstamped, flat pipelines map) and ForStampedEntrypoints (stamp-expanded, stamp→pipelines map). Callers that don't use stamps no longer see the map[string]map[string]*Pipeline type. ForEntrypoint remains as a single-entrypoint convenience wrapper. Extract resolveIterations, accumulateIteration, registerResourceGroups, and wireInterServiceEdges from the monolithic accumulate method. Add stamp and regionShort parameters to GetRegionConfiguration and ValueProvenance so they re-template per-stamp instead of using the resolver's initial stamp. Extract resolveOverrides to share the preprocessing logic. --- config/config.go | 76 ++- config/config_test.go | 67 ++- config/ev2config/config.go | 11 + .../zz_fixture_TestConfigProvider.yaml | 8 +- pipelines/graph/graph.go | 446 ++++++++++-------- pipelines/graph/graph_stamped_test.go | 188 +++++++- pipelines/graph/graph_test.go | 46 ++ .../zz_fixture_TestNewPipelineFromFile.yaml | 2 +- 8 files changed, 606 insertions(+), 238 deletions(-) diff --git a/config/config.go b/config/config.go index 7b826c1b..baa09a5c 100644 --- a/config/config.go +++ b/config/config.go @@ -82,11 +82,15 @@ type ConfigResolver interface { GetRegions() ([]string, error) // GetRegionConfiguration resolves the configuration for a region in the cloud and environment. // It re-processes the configuration template with the given region and stamp. - GetRegionConfiguration(region, stamp string) (types.Configuration, error) + // The regionShort for the region is derived from the embedded ev2 configuration. When an + // explicit regionShort override is provided, it is used instead of the derived value. + GetRegionConfiguration(region, stamp string, regionShort ...string) (types.Configuration, error) // GetRegionOverrides fetches the overrides specific to a region, if any exist. GetRegionOverrides(region string) (types.Configuration, error) // ValueProvenance divulges how the value at 'path' is overridden to arrive at the result. - ValueProvenance(region, path string) (*Provenance, error) + // The regionShort for the region is derived from the embedded ev2 configuration. When an + // explicit regionShort override is provided, it is used instead of the derived value. + ValueProvenance(region, stamp, path string, regionShort ...string) (*Provenance, error) } // NewConfigProvider creates a configuration provider by knowing the path to the configuration file. @@ -265,24 +269,54 @@ func (cr *configResolver) GetRegions() ([]string, error) { return regions, nil } -// GetRegionConfiguration re-templates the configuration with the given region and stamp, -// then merges values to resolve the configuration for the region. -func (cr *configResolver) GetRegionConfiguration(region, stamp string) (types.Configuration, error) { +func (cr *configResolver) resolveOverrides(region, stamp string, regionShort ...string) (configurationOverrides, error) { replacements := cr.replacements replacements.RegionReplacement = region replacements.StampReplacement = stamp + + ev2Cfg, err := ev2config.ResolveConfig(cr.cloud, region) + if err != nil { + return configurationOverrides{}, fmt.Errorf("failed to resolve ev2 config for region %s: %w", region, err) + } + replacements.Ev2Config = ev2Cfg + + if len(regionShort) > 0 && regionShort[0] != "" { + replacements.RegionShortReplacement = regionShort[0] + } else { + rawShort, err := ev2Cfg.GetByPath("regionShortName") + if err != nil { + return configurationOverrides{}, fmt.Errorf("regionShortName not found in ev2 config for region %s: %w", region, err) + } + short, ok := rawShort.(string) + if !ok { + return configurationOverrides{}, fmt.Errorf("regionShortName for region %s is %T, not string", region, rawShort) + } + replacements.RegionShortReplacement = short + } + rawContent, err := PreprocessContent(cr.provider.raw, replacements.AsMap()) if err != nil { - return nil, fmt.Errorf("failed to preprocess config for region %s stamp %s: %w", region, stamp, err) + return configurationOverrides{}, fmt.Errorf("failed to preprocess config for region %s stamp %s: %w", region, stamp, err) } - regionalOverrides := configurationOverrides{} - if err := yaml.Unmarshal(rawContent, ®ionalOverrides); err != nil { - return nil, fmt.Errorf("failed to unmarshal config for region %s stamp %s: %w", region, stamp, err) + overrides := configurationOverrides{} + if err := yaml.Unmarshal(rawContent, &overrides); err != nil { + return configurationOverrides{}, fmt.Errorf("failed to unmarshal config for region %s stamp %s: %w", region, stamp, err) + } + + return overrides, nil +} + +// GetRegionConfiguration re-templates the configuration with the given region and stamp, +// then merges values to resolve the configuration for the region. +func (cr *configResolver) GetRegionConfiguration(region, stamp string, regionShort ...string) (types.Configuration, error) { + overrides, err := cr.resolveOverrides(region, stamp, regionShort...) + if err != nil { + return nil, err } - cfg := regionalOverrides.Defaults - cloudCfg, hasCloud := regionalOverrides.Overrides[cr.cloud] + cfg := overrides.Defaults + cloudCfg, hasCloud := overrides.Overrides[cr.cloud] if !hasCloud { return nil, fmt.Errorf("the cloud %s is not found in the config", cr.cloud) } @@ -354,8 +388,13 @@ type Provenance struct { // ValueProvenance determines the provenance of a value in the configuration - which levels of overrides have something to do // with this value, how do they override each other, what is the resulting value? -func (cr *configResolver) ValueProvenance(region, path string) (*Provenance, error) { - cloudCfg, hasCloud := cr.cfg.Overrides[cr.cloud] +func (cr *configResolver) ValueProvenance(region, stamp, path string, regionShort ...string) (*Provenance, error) { + overrides, err := cr.resolveOverrides(region, stamp, regionShort...) + if err != nil { + return nil, err + } + + cloudCfg, hasCloud := overrides.Overrides[cr.cloud] if !hasCloud { return nil, fmt.Errorf("the cloud %s is not found in the config", cr.cloud) } @@ -365,14 +404,13 @@ func (cr *configResolver) ValueProvenance(region, path string) (*Provenance, err } regionCfg, hasRegion := envCfg.Overrides[region] if !hasRegion { - // a missing region just means we use default values regionCfg = types.Configuration{} } - mergedCfg, err := cr.GetRegionConfiguration(region, cr.replacements.StampReplacement) - if err != nil { - return nil, err - } + mergedCfg := overrides.Defaults + mergedCfg = types.MergeConfiguration(mergedCfg, cloudCfg.Defaults) + mergedCfg = types.MergeConfiguration(mergedCfg, envCfg.Defaults) + mergedCfg = types.MergeConfiguration(mergedCfg, regionCfg) p := &Provenance{} for name, part := range map[string]struct { @@ -380,7 +418,7 @@ func (cr *configResolver) ValueProvenance(region, path string) (*Provenance, err value *any set *bool }{ - "default": {from: &cr.cfg.Defaults, value: &p.Default, set: &p.DefaultSet}, + "default": {from: &overrides.Defaults, value: &p.Default, set: &p.DefaultSet}, "cloud": {from: &cloudCfg.Defaults, value: &p.Cloud, set: &p.CloudSet}, "environment": {from: &envCfg.Defaults, value: &p.Environment, set: &p.EnvironmentSet}, "region": {from: ®ionCfg, value: &p.Region, set: &p.RegionSet}, diff --git a/config/config_test.go b/config/config_test.go index e24dbb74..fb181ea8 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -85,17 +85,17 @@ func TestGetRegionStampConfiguration(t *testing.T) { { name: "stamp 1", stamp: "1", - expectedMgmtRG: "hcp-underlay-uks-mgmt-1", + expectedMgmtRG: "hcp-underlay-ln-mgmt-1", }, { name: "stamp 2", stamp: "2", - expectedMgmtRG: "hcp-underlay-uks-mgmt-2", + expectedMgmtRG: "hcp-underlay-ln-mgmt-2", }, { name: "stamp 3", stamp: "3", - expectedMgmtRG: "hcp-underlay-uks-mgmt-3", + expectedMgmtRG: "hcp-underlay-ln-mgmt-3", }, } @@ -134,7 +134,7 @@ func TestConfigProvenance(t *testing.T) { }) require.NoError(t, err) - ubiquitous, err := configResolver.ValueProvenance(region, "ubiquitousValue") + ubiquitous, err := configResolver.ValueProvenance(region, stamp, "ubiquitousValue") require.NoError(t, err) if diff := cmp.Diff(ubiquitous, &config.Provenance{ @@ -152,7 +152,7 @@ func TestConfigProvenance(t *testing.T) { t.Errorf("Provenance mismatch for ubiquitousValue (-want +got):\n%s", diff) } - partial, err := configResolver.ValueProvenance(region, "partialValue") + partial, err := configResolver.ValueProvenance(region, stamp, "partialValue") require.NoError(t, err) if diff := cmp.Diff(partial, &config.Provenance{ @@ -171,6 +171,63 @@ func TestConfigProvenance(t *testing.T) { } } +func TestConfigProvenanceReTemplatesForStamp(t *testing.T) { + region := "uksouth" + regionShort := "uks" + cloud := "public" + environment := "int" + + ev2, err := ev2config.ResolveConfig(cloud, region) + require.NoError(t, err) + + configProvider, err := config.NewConfigProvider("./testdata/pipelines/config.yaml") + require.NoError(t, err) + configResolver, err := configProvider.GetResolver(&config.ConfigReplacements{ + RegionReplacement: region, + RegionShortReplacement: regionShort, + StampReplacement: "1", + CloudReplacement: cloud, + EnvironmentReplacement: environment, + Ev2Config: ev2, + }) + require.NoError(t, err) + + testCases := []struct { + name string + stamp string + expectedDefaultRG string + expectedResultRG string + }{ + { + name: "stamp 1", + stamp: "1", + expectedDefaultRG: "hcp-underlay-ln-mgmt-1", + expectedResultRG: "hcp-underlay-ln-mgmt-1", + }, + { + name: "stamp 2", + stamp: "2", + expectedDefaultRG: "hcp-underlay-ln-mgmt-2", + expectedResultRG: "hcp-underlay-ln-mgmt-2", + }, + { + name: "stamp 3", + stamp: "3", + expectedDefaultRG: "hcp-underlay-ln-mgmt-3", + expectedResultRG: "hcp-underlay-ln-mgmt-3", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + p, err := configResolver.ValueProvenance(region, tc.stamp, "managementClusterRG") + require.NoError(t, err) + require.Equal(t, tc.expectedDefaultRG, p.Default) + require.Equal(t, tc.expectedResultRG, p.Result) + }) + } +} + func TestMergeConfiguration(t *testing.T) { testCases := []struct { name string diff --git a/config/ev2config/config.go b/config/ev2config/config.go index 2cb45c05..a6e23233 100644 --- a/config/ev2config/config.go +++ b/config/ev2config/config.go @@ -11,6 +11,11 @@ import ( _ "embed" ) +const ( + cloudDev = "dev" + cloudPublic = "public" +) + //go:embed config.yaml var rawConfig []byte @@ -44,6 +49,9 @@ func ResolveConfig(cloud, region string) (types.Configuration, error) { } cfg := types.Configuration{} cloudCfg, hasCloud := ev2Config.Clouds[cloud] + if !hasCloud && cloud == cloudDev { + cloudCfg, hasCloud = ev2Config.Clouds[cloudPublic] + } if !hasCloud { return nil, fmt.Errorf("failed to find cloud %s", cloud) } @@ -63,6 +71,9 @@ func ResolveConfigForCloud(cloud string) (types.Configuration, error) { } cloudCfg, hasCloud := ev2Config.Clouds[cloud] + if !hasCloud && cloud == cloudDev { + cloudCfg, hasCloud = ev2Config.Clouds[cloudPublic] + } if !hasCloud { return nil, fmt.Errorf("failed to find cloud %s", cloud) } diff --git a/config/testdata/zz_fixture_TestConfigProvider.yaml b/config/testdata/zz_fixture_TestConfigProvider.yaml index 1a1b396b..395f3e88 100644 --- a/config/testdata/zz_fixture_TestConfigProvider.yaml +++ b/config/testdata/zz_fixture_TestConfigProvider.yaml @@ -39,21 +39,21 @@ imageMirror: adoProject: adoProject artifactName: artifactName buildId: 12345 -imageSyncRG: hcp-underlay-uks-imagesync +imageSyncRG: hcp-underlay-ln-imagesync kusto: cluster: aroINT resourceGroup: aro-kusto-public-int-us serviceLogsDatabase: containerLogs maestro_helm_chart: oci://aro-hcp-int.azurecr.io/helm/server maestro_image: aro-hcp-int.azurecr.io/maestro-server:the-stable-one -managementClusterRG: hcp-underlay-uks-mgmt-1 +managementClusterRG: hcp-underlay-ln-mgmt-1 managementClusterSubscription: hcp-uksouth parentZone: example.com partialValue: public-int-uksouth-value provider: Self region: uksouth -regionRG: hcp-underlay-uks -serviceClusterRG: hcp-underlay-uks-svc +regionRG: hcp-underlay-ln +serviceClusterRG: hcp-underlay-ln-svc serviceClusterSubscription: hcp-uksouth storage: accountName: arotestaccount diff --git a/pipelines/graph/graph.go b/pipelines/graph/graph.go index 4c40e8c5..0f4a6cea 100644 --- a/pipelines/graph/graph.go +++ b/pipelines/graph/graph.go @@ -40,7 +40,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. @@ -61,7 +61,7 @@ func newGraph() *Graph { return &Graph{ Services: map[string]*topology.Service{}, ResourceGroups: map[ResourceGroupKey]*types.ResourceGroupMeta{}, - resourceGroupOwners: map[string]sets.Set[string]{}, + resourceGroupOwners: map[ResourceGroupKey]sets.Set[string]{}, Steps: map[Identifier]types.Step{}, Nodes: []Node{}, ServiceValidationSteps: map[Identifier]types.ValidationStep{}, @@ -91,7 +91,7 @@ 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] } // GetResourceGroup returns the resource group metadata for the given key. @@ -126,6 +126,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 an empty stamp. +type stampIteration struct { + stamp string + 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{ @@ -150,14 +157,29 @@ func ForPipeline(service *topology.Service, pipeline *types.Pipeline) (*Graph, e } // 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}, map[string]map[string]*types.Pipeline{"": pipelines}) + 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. -// stampPipelines maps stamp identifiers to per-service-group pipelines. Callers that do not use stamps pass a single -// entry keyed by "" (empty string). When stamped services are encountered, the graph expands them once per stamp. -func ForEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, stampPipelines map[string]map[string]*types.Pipeline) (*Graph, error) { +// Stamped services in the topology are not expanded — each appears exactly once with an empty Stamp field 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[string]map[string]*types.Pipeline{"": 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 non-empty key in stampPipelines — with each copy carrying a distinct Stamp field on its identifiers. +// Unstamped services appear once with an empty Stamp. 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[string]map[string]*types.Pipeline) (*Graph, error) { + return forEntrypoints(topo, entrypoints, stampPipelines) +} + +func forEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, stampPipelines map[string]map[string]*types.Pipeline) (*Graph, error) { var roots []*topology.Service for _, entrypoint := range entrypoints { root, err := topo.Lookup(entrypoint.Identifier) @@ -176,9 +198,9 @@ func ForEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, } // 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. + // 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 := graph.addExternalDependencyEdges(); err != nil { return nil, err } @@ -186,174 +208,228 @@ func ForEntrypoints(topo *topology.Topology, entrypoints []*topology.Entrypoint, return graph, graph.detectCycles() } -// accumulate recursively traverses the service and all children, building a graph of how steps in each service -// depend on each other. When a service is stamped, it expands nodes once per stamp; unstamped services are -// processed once with an empty stamp. +// 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 *Graph) accumulate(service *topology.Service, stampPipelines map[string]map[string]*types.Pipeline) error { if _, alreadyRecorded := c.Services[service.ServiceGroup]; alreadyRecorded { return fmt.Errorf("service group %s already recorded", service.ServiceGroup) } - c.Services[service.ServiceGroup] = service - // Determine which stamp iterations to run: stamped services expand once per stamp key, - // unstamped services run once with an empty stamp. - type stampIteration struct { - stamp string - pipeline *types.Pipeline + iterations, err := resolveIterations(service, stampPipelines) + if err != nil { + return err } - var iterations []stampIteration - if service.IsStamped() { - for _, stamp := range slices.Sorted(maps.Keys(stampPipelines)) { - if stamp == "" { - continue - } - pipeline, exists := stampPipelines[stamp][service.ServiceGroup] - if !exists { - return fmt.Errorf("pipeline for service %s not found in stamp %s", service.ServiceGroup, stamp) - } - iterations = append(iterations, stampIteration{stamp: stamp, pipeline: pipeline}) + + // Leaf nodes are collected per stamp so that inter-service wiring connects only matching stamps. + allLeaves := map[string][]Identifier{} + for _, iter := range iterations { + leaves, err := c.accumulateIteration(service.ServiceGroup, iter) + if err != nil { + return err } - if len(iterations) == 0 { - return fmt.Errorf("stamped service %s requires at least one non-empty stamp key, but no non-empty stamp keys were provided", service.ServiceGroup) + allLeaves[iter.stamp] = leaves + } + + // 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 } - } else { - // Unstamped services have the same pipeline regardless of stamp key — grab from any entry. - var pipeline *types.Pipeline + if err := c.wireInterServiceEdges(service, &child, allLeaves); err != nil { + return err + } + } + + return nil +} + +// resolveIterations determines which stamp/pipeline pairs to process for a service. +// Unstamped services run once with an empty stamp. Stamped services expand once per +// non-empty stamp key. +func resolveIterations(service *topology.Service, stampPipelines map[string]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 { - pipeline = p - break + return []stampIteration{{pipeline: p}}, nil } } - if pipeline == nil { - return fmt.Errorf("pipeline for service %s not found", service.ServiceGroup) - } - iterations = append(iterations, stampIteration{stamp: "", pipeline: pipeline}) + return nil, fmt.Errorf("pipeline for service %s not found", service.ServiceGroup) } - // For each stamp iteration, generate nodes and register resource groups, steps, and metadata. - // Leaf nodes are collected per stamp so that inter-service wiring connects only matching stamps. - allLeaves := map[string][]Identifier{} - for _, iter := range iterations { - resourceGroups, subscription, steps, serviceValidationSteps, nodes, err := nodesFor(iter.pipeline, iter.stamp) - if err != nil { - return fmt.Errorf("failed to generate graph for pipeline %s: %v", service.ServiceGroup, err) + var stamps []string + for stamp := range stampPipelines { + if stamp != "" { + stamps = append(stamps, stamp) } + } + slices.Sort(stamps) - // Register resource groups, detecting conflicts within the same stamp and across stamp boundaries. - for name, group := range resourceGroups { - key := ResourceGroupKey{Stamp: iter.stamp, Name: name} - other, alreadyRecorded := c.ResourceGroups[key] - if alreadyRecorded && !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)) - } - if alreadyRecorded { - c.resourceGroupOwners[name].Insert(service.ServiceGroup) - } else { - c.ResourceGroups[key] = group - c.resourceGroupOwners[name] = sets.New(service.ServiceGroup) + // No non-empty 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 } } + 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) - } - c.Subscription = subscription + 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 +} - // 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: service.ServiceGroup, StepDependency: types.StepDependency{ResourceGroup: rg, Step: stepName}}] = step - } +// accumulateIteration processes one stamp iteration: generates nodes, registers resource groups, +// steps, and metadata, and returns the leaf nodes for inter-service wiring. +func (c *Graph) 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 + } + + // 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) - c.Nodes = append(c.Nodes, nodes...) + } + maps.Copy(c.ServiceValidationSteps, serviceValidationSteps) + c.Nodes = append(c.Nodes, nodes...) - // Identify leaf nodes for this stamp iteration — these will become parents of child service roots. - 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) - } - 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) - } - } + return c.findLeaves(nodes) +} + +// registerResourceGroups records resource groups, detecting conflicts across services and stamps. +func (c *Graph) registerResourceGroups(serviceGroup, stamp string, 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)) } - allLeaves[iter.stamp] = leaves + if !alreadyRecorded { + c.ResourceGroups[key] = group + c.resourceGroupOwners[key] = sets.New[string]() + } + c.resourceGroupOwners[key].Insert(serviceGroup) } + return nil +} - // 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 +// 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 *Graph) findLeaves(nodes []Node) ([]Identifier, error) { + var leaves []Identifier + for _, node := range nodes { + _, _, step, err := c.lookup(node.Identifier) + if err != nil { + return nil, fmt.Errorf("failed to lookup node: %v", err) + } + 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 +} - // 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. - // - // When stamps are involved, wiring is stamp-scoped: stamp-1 leaves connect to stamp-1 roots only. - // Unstamped leaves (stamp="") connect to all child roots that share the same stamp="" or fan out - // to all stamps when the child is stamped and the parent is not. - - for parentStamp, leaves := range allLeaves { - var roots []Identifier - // Find root nodes 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. - for i, 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 service.IsStamped() && child.IsStamped() && node.Stamp != parentStamp { - continue - } - roots = append(roots, node.Identifier) - c.Nodes[i].Parents = append(c.Nodes[i].Parents, leaves...) +func (c *Graph) 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 +} + +// 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 (stamp="") connect to all child roots that share the same stamp="" or fan out +// to all stamps when the child is stamped and the parent is not. +func (c *Graph) wireInterServiceEdges(parent *topology.Service, child *topology.Service, allLeaves map[string][]Identifier) error { + for parentStamp, leaves := range allLeaves { + roots := c.findChildRoots(parent, child, parentStamp) + + for i, node := range c.Nodes { + if !slices.Contains(roots, node.Identifier) { + continue } + c.Nodes[i].Parents = append(c.Nodes[i].Parents, leaves...) + } - for i, node := range c.Nodes { - for _, leaf := range leaves { - if node.ServiceGroup == leaf.ServiceGroup && node.ResourceGroup == leaf.ResourceGroup && node.Step == leaf.Step && node.Stamp == leaf.Stamp { - 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, roots...) } } - 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 *Graph) findChildRoots(parent, child *topology.Service, parentStamp string) []Identifier { + var roots []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 = append(roots, 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 { @@ -397,10 +473,14 @@ func (c *Graph) addExternalDependencyEdges() error { Step: dep.Step, }, } - // External dependencies don't carry stamp information. When the target service is stamped, - // resolve to the same stamp as the node declaring the dependency — stamp-1 work depends on - // stamp-1 of the target, not all stamps. Unstamped targets keep an empty stamp. + // 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) @@ -437,10 +517,10 @@ func nodesFor(pipeline *types.Pipeline, stamp string) ( 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, @@ -500,30 +580,29 @@ func nodesFor(pipeline *types.Pipeline, stamp string) ( 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{ - Stamp: stamp, - 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], + }) } } @@ -564,17 +643,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 } } @@ -584,18 +664,19 @@ 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 { if slices.Contains(seen, child) { var cycle []string @@ -604,20 +685,11 @@ func traverse(node Node, all []Node, seen []Identifier) error { } return fmt.Errorf("cycle detected, reached %s via %s", child, strings.Join(cycle, " -> ")) } - chain := seen[:] - chain = append(chain, child) - var childNode Node - var found bool - for _, candidate := range all { - if candidate.Identifier == child { - childNode = candidate - found = true - } - } + 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 } } diff --git a/pipelines/graph/graph_stamped_test.go b/pipelines/graph/graph_stamped_test.go index 0dcde3b8..70b5f9f1 100644 --- a/pipelines/graph/graph_stamped_test.go +++ b/pipelines/graph/graph_stamped_test.go @@ -6,6 +6,7 @@ import ( "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" @@ -89,12 +90,12 @@ func stampSets(nodes []Node) []string { return stamps } -// buildAndValidate constructs a graph via ForEntrypoints and runs the validation function against it. +// buildAndValidate constructs a graph via ForStampedEntrypoints and runs the validation function against it. func buildAndValidate(t *testing.T, topo *topology.Topology, stampPipelines map[string]map[string]*types.Pipeline, validate func(t *testing.T, result *Graph)) { t.Helper() entrypoint := &topo.Entrypoints[0] - result, err := ForEntrypoints(topo, []*topology.Entrypoint{entrypoint}, stampPipelines) - assert.NoError(t, err) + result, err := ForStampedEntrypoints(topo, []*topology.Entrypoint{entrypoint}, stampPipelines) + require.NoError(t, err) validate(t, result) } @@ -470,6 +471,7 @@ func TestStampedExternalDeps(t *testing.T) { name string topo *topology.Topology stampPipelines map[string]map[string]*types.Pipeline + expectErr string validate func(t *testing.T, result *Graph) }{ { @@ -570,11 +572,78 @@ func TestStampedExternalDeps(t *testing.T) { 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[string]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[string]map[string]*types.Pipeline{ + "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), + }, + "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[string]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[string]map[string]*types.Pipeline{ + "": { + "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) { - buildAndValidate(t, testCase.topo, testCase.stampPipelines, testCase.validate) + 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) }) } } @@ -611,29 +680,104 @@ func TestDetectCyclesStamped(t *testing.T) { assert.False(t, hasStamp1 && hasStamp2, "cycle path should stay within one stamp, got: %s", msg) } -// TestStampedServiceRequiresNonEmptyStampKey verifies that stamped services fail loudly -// when no non-empty stamp key is provided. -func TestStampedServiceRequiresNonEmptyStampKey(t *testing.T) { +// 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"}} - 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"}, + 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), + }, }, - }) - - stampPipelines := map[string]map[string]*types.Pipeline{ - "": { - "SG.Infra": makePipeline("SG.Infra", "infra-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.Empty(t, dbParents[0].Stamp, "external dep in no-expansion mode has empty stamp") + }, }, } - entrypoint := &topo.Entrypoints[0] - _, err := ForEntrypoints(topo, []*topology.Entrypoint{entrypoint}, stampPipelines) - assert.Error(t, err) - assert.Contains(t, err.Error(), "no non-empty stamp keys") + 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.Empty(t, mgmtNodes[0].Stamp) + + if testCase.validate != nil { + testCase.validate(t, result) + } + }) + } } // TestForPipelineStampedService verifies that ForPipeline works with a stamped service, @@ -694,7 +838,7 @@ func TestStampedEntrypointDOT(t *testing.T) { } entrypoint := &topo.Entrypoints[0] - result, err := ForEntrypoints(topo, []*topology.Entrypoint{entrypoint}, stampPipelines) + result, err := ForStampedEntrypoints(topo, []*topology.Entrypoint{entrypoint}, stampPipelines) assert.NoError(t, err) encoded, err := MarshalDOT(result) diff --git a/pipelines/graph/graph_test.go b/pipelines/graph/graph_test.go index 8e3aafb8..db580bd6 100644 --- a/pipelines/graph/graph_test.go +++ b/pipelines/graph/graph_test.go @@ -105,6 +105,52 @@ 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", + }, + } + + g := &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 := g.findLeaves(g.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/types/testdata/zz_fixture_TestNewPipelineFromFile.yaml b/pipelines/types/testdata/zz_fixture_TestNewPipelineFromFile.yaml index 001a65bf..623aafd2 100644 --- a/pipelines/types/testdata/zz_fixture_TestNewPipelineFromFile.yaml +++ b/pipelines/types/testdata/zz_fixture_TestNewPipelineFromFile.yaml @@ -20,7 +20,7 @@ resourceGroups: - uksouth singleton: false name: regional - resourceGroup: hcp-underlay-uks + resourceGroup: hcp-underlay-ln steps: - action: Shell adoArtifacts: From 74e83f2752e27c7084215ead00bf21f4743fdae1 Mon Sep 17 00:00:00 2001 From: Gerd Oberlechner Date: Wed, 24 Jun 2026 15:33:51 +0200 Subject: [PATCH 3/3] revert stamped config resolver change Signed-off-by: Gerd Oberlechner --- config/config.go | 87 +++---------- config/config_internal_test.go | 6 +- config/config_test.go | 119 +----------------- config/ev2config/config.go | 11 -- .../zz_fixture_TestConfigProvider.yaml | 8 +- pipelines/types/pipeline_test.go | 4 +- .../zz_fixture_TestNewPipelineFromFile.yaml | 2 +- 7 files changed, 28 insertions(+), 209 deletions(-) diff --git a/config/config.go b/config/config.go index baa09a5c..36ea5e67 100644 --- a/config/config.go +++ b/config/config.go @@ -81,16 +81,11 @@ type ConfigResolver interface { // GetRegions divulges the regions for which overrides are registered. GetRegions() ([]string, error) // GetRegionConfiguration resolves the configuration for a region in the cloud and environment. - // It re-processes the configuration template with the given region and stamp. - // The regionShort for the region is derived from the embedded ev2 configuration. When an - // explicit regionShort override is provided, it is used instead of the derived value. - GetRegionConfiguration(region, stamp string, regionShort ...string) (types.Configuration, error) + GetRegionConfiguration(region string) (types.Configuration, error) // GetRegionOverrides fetches the overrides specific to a region, if any exist. GetRegionOverrides(region string) (types.Configuration, error) // ValueProvenance divulges how the value at 'path' is overridden to arrive at the result. - // The regionShort for the region is derived from the embedded ev2 configuration. When an - // explicit regionShort override is provided, it is used instead of the derived value. - ValueProvenance(region, stamp, path string, regionShort ...string) (*Provenance, error) + ValueProvenance(region, path string) (*Provenance, error) } // NewConfigProvider creates a configuration provider by knowing the path to the configuration file. @@ -209,8 +204,6 @@ func (cp *configProvider) GetResolver(configReplacements *ConfigReplacements) (C environment: configReplacements.EnvironmentReplacement, cfg: currentVariableOverrides, absoluteSchemaPath: cp.absoluteSchemaPath, - provider: cp, - replacements: *configReplacements, }, nil } @@ -218,9 +211,6 @@ type configResolver struct { cloud, environment string cfg configurationOverrides absoluteSchemaPath string - - provider *configProvider - replacements ConfigReplacements } func (cr *configResolver) ValidateSchema(config types.Configuration) error { @@ -269,54 +259,10 @@ func (cr *configResolver) GetRegions() ([]string, error) { return regions, nil } -func (cr *configResolver) resolveOverrides(region, stamp string, regionShort ...string) (configurationOverrides, error) { - replacements := cr.replacements - replacements.RegionReplacement = region - replacements.StampReplacement = stamp - - ev2Cfg, err := ev2config.ResolveConfig(cr.cloud, region) - if err != nil { - return configurationOverrides{}, fmt.Errorf("failed to resolve ev2 config for region %s: %w", region, err) - } - replacements.Ev2Config = ev2Cfg - - if len(regionShort) > 0 && regionShort[0] != "" { - replacements.RegionShortReplacement = regionShort[0] - } else { - rawShort, err := ev2Cfg.GetByPath("regionShortName") - if err != nil { - return configurationOverrides{}, fmt.Errorf("regionShortName not found in ev2 config for region %s: %w", region, err) - } - short, ok := rawShort.(string) - if !ok { - return configurationOverrides{}, fmt.Errorf("regionShortName for region %s is %T, not string", region, rawShort) - } - replacements.RegionShortReplacement = short - } - - rawContent, err := PreprocessContent(cr.provider.raw, replacements.AsMap()) - if err != nil { - return configurationOverrides{}, fmt.Errorf("failed to preprocess config for region %s stamp %s: %w", region, stamp, err) - } - - overrides := configurationOverrides{} - if err := yaml.Unmarshal(rawContent, &overrides); err != nil { - return configurationOverrides{}, fmt.Errorf("failed to unmarshal config for region %s stamp %s: %w", region, stamp, err) - } - - return overrides, nil -} - -// GetRegionConfiguration re-templates the configuration with the given region and stamp, -// then merges values to resolve the configuration for the region. -func (cr *configResolver) GetRegionConfiguration(region, stamp string, regionShort ...string) (types.Configuration, error) { - overrides, err := cr.resolveOverrides(region, stamp, regionShort...) - if err != nil { - return nil, err - } - - cfg := overrides.Defaults - cloudCfg, hasCloud := overrides.Overrides[cr.cloud] +// GetRegionConfiguration merges values to resolve the configuration for a region. +func (cr *configResolver) GetRegionConfiguration(region string) (types.Configuration, error) { + cfg := cr.cfg.Defaults + cloudCfg, hasCloud := cr.cfg.Overrides[cr.cloud] if !hasCloud { return nil, fmt.Errorf("the cloud %s is not found in the config", cr.cloud) } @@ -328,6 +274,7 @@ func (cr *configResolver) GetRegionConfiguration(region, stamp string, regionSho cfg = types.MergeConfiguration(cfg, envCfg.Defaults) regionCfg, hasRegion := envCfg.Overrides[region] if !hasRegion { + // a missing region just means we use default values regionCfg = types.Configuration{} } cfg = types.MergeConfiguration(cfg, regionCfg) @@ -388,13 +335,8 @@ type Provenance struct { // ValueProvenance determines the provenance of a value in the configuration - which levels of overrides have something to do // with this value, how do they override each other, what is the resulting value? -func (cr *configResolver) ValueProvenance(region, stamp, path string, regionShort ...string) (*Provenance, error) { - overrides, err := cr.resolveOverrides(region, stamp, regionShort...) - if err != nil { - return nil, err - } - - cloudCfg, hasCloud := overrides.Overrides[cr.cloud] +func (cr *configResolver) ValueProvenance(region, path string) (*Provenance, error) { + cloudCfg, hasCloud := cr.cfg.Overrides[cr.cloud] if !hasCloud { return nil, fmt.Errorf("the cloud %s is not found in the config", cr.cloud) } @@ -404,13 +346,14 @@ func (cr *configResolver) ValueProvenance(region, stamp, path string, regionShor } regionCfg, hasRegion := envCfg.Overrides[region] if !hasRegion { + // a missing region just means we use default values regionCfg = types.Configuration{} } - mergedCfg := overrides.Defaults - mergedCfg = types.MergeConfiguration(mergedCfg, cloudCfg.Defaults) - mergedCfg = types.MergeConfiguration(mergedCfg, envCfg.Defaults) - mergedCfg = types.MergeConfiguration(mergedCfg, regionCfg) + mergedCfg, err := cr.GetRegionConfiguration(region) + if err != nil { + return nil, err + } p := &Provenance{} for name, part := range map[string]struct { @@ -418,7 +361,7 @@ func (cr *configResolver) ValueProvenance(region, stamp, path string, regionShor value *any set *bool }{ - "default": {from: &overrides.Defaults, value: &p.Default, set: &p.DefaultSet}, + "default": {from: &cr.cfg.Defaults, value: &p.Default, set: &p.DefaultSet}, "cloud": {from: &cloudCfg.Defaults, value: &p.Cloud, set: &p.CloudSet}, "environment": {from: &envCfg.Defaults, value: &p.Environment, set: &p.EnvironmentSet}, "region": {from: ®ionCfg, value: &p.Region, set: &p.RegionSet}, diff --git a/config/config_internal_test.go b/config/config_internal_test.go index 3f974447..e77c112d 100644 --- a/config/config_internal_test.go +++ b/config/config_internal_test.go @@ -33,7 +33,7 @@ func TestValidateSchema(t *testing.T) { }) require.NoError(t, err) - cfg, err := resolver.GetRegionConfiguration("uksouth", "1") + cfg, err := resolver.GetRegionConfiguration("uksouth") require.NoError(t, err) validationErr := resolver.ValidateSchema(cfg) @@ -55,7 +55,7 @@ func TestValidateSchemaWithCEL(t *testing.T) { }) require.NoError(t, err) - cfg, err := resolver.GetRegionConfiguration("uksouth", "1") + cfg, err := resolver.GetRegionConfiguration("uksouth") require.NoError(t, err) validationErr := resolver.ValidateSchema(cfg) @@ -75,7 +75,7 @@ func TestValidateSchemaWithCEL(t *testing.T) { }) require.NoError(t, err) - cfg, err := resolver.GetRegionConfiguration("uksouth", "1") + cfg, err := resolver.GetRegionConfiguration("uksouth") require.NoError(t, err) validationErr := resolver.ValidateSchema(cfg) diff --git a/config/config_test.go b/config/config_test.go index fb181ea8..eb64e5df 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -49,69 +49,13 @@ func TestConfigProvider(t *testing.T) { }) require.NoError(t, err) - cfg, err := configResolver.GetRegionConfiguration(region, stamp) + cfg, err := configResolver.GetRegionConfiguration(region) require.NoError(t, err) require.NotNil(t, cfg) testutil.CompareWithFixture(t, cfg) } -func TestGetRegionStampConfiguration(t *testing.T) { - region := "uksouth" - regionShort := "uks" - cloud := "public" - environment := "int" - - ev2, err := ev2config.ResolveConfig(cloud, region) - require.NoError(t, err) - - configProvider, err := config.NewConfigProvider("./testdata/pipelines/config.yaml") - require.NoError(t, err) - configResolver, err := configProvider.GetResolver(&config.ConfigReplacements{ - RegionReplacement: region, - RegionShortReplacement: regionShort, - StampReplacement: "1", - CloudReplacement: cloud, - EnvironmentReplacement: environment, - Ev2Config: ev2, - }) - require.NoError(t, err) - - testCases := []struct { - name string - stamp string - expectedMgmtRG string - }{ - { - name: "stamp 1", - stamp: "1", - expectedMgmtRG: "hcp-underlay-ln-mgmt-1", - }, - { - name: "stamp 2", - stamp: "2", - expectedMgmtRG: "hcp-underlay-ln-mgmt-2", - }, - { - name: "stamp 3", - stamp: "3", - expectedMgmtRG: "hcp-underlay-ln-mgmt-3", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - cfg, err := configResolver.GetRegionConfiguration(region, tc.stamp) - require.NoError(t, err) - require.NotNil(t, cfg) - - mgmtRG, ok := cfg["managementClusterRG"] - require.True(t, ok, "managementClusterRG should be present in config") - require.Equal(t, tc.expectedMgmtRG, mgmtRG) - }) - } -} - func TestConfigProvenance(t *testing.T) { region := "uksouth" regionShort := "uks" @@ -134,7 +78,7 @@ func TestConfigProvenance(t *testing.T) { }) require.NoError(t, err) - ubiquitous, err := configResolver.ValueProvenance(region, stamp, "ubiquitousValue") + ubiquitous, err := configResolver.ValueProvenance(region, "ubiquitousValue") require.NoError(t, err) if diff := cmp.Diff(ubiquitous, &config.Provenance{ @@ -152,7 +96,7 @@ func TestConfigProvenance(t *testing.T) { t.Errorf("Provenance mismatch for ubiquitousValue (-want +got):\n%s", diff) } - partial, err := configResolver.ValueProvenance(region, stamp, "partialValue") + partial, err := configResolver.ValueProvenance(region, "partialValue") require.NoError(t, err) if diff := cmp.Diff(partial, &config.Provenance{ @@ -171,63 +115,6 @@ func TestConfigProvenance(t *testing.T) { } } -func TestConfigProvenanceReTemplatesForStamp(t *testing.T) { - region := "uksouth" - regionShort := "uks" - cloud := "public" - environment := "int" - - ev2, err := ev2config.ResolveConfig(cloud, region) - require.NoError(t, err) - - configProvider, err := config.NewConfigProvider("./testdata/pipelines/config.yaml") - require.NoError(t, err) - configResolver, err := configProvider.GetResolver(&config.ConfigReplacements{ - RegionReplacement: region, - RegionShortReplacement: regionShort, - StampReplacement: "1", - CloudReplacement: cloud, - EnvironmentReplacement: environment, - Ev2Config: ev2, - }) - require.NoError(t, err) - - testCases := []struct { - name string - stamp string - expectedDefaultRG string - expectedResultRG string - }{ - { - name: "stamp 1", - stamp: "1", - expectedDefaultRG: "hcp-underlay-ln-mgmt-1", - expectedResultRG: "hcp-underlay-ln-mgmt-1", - }, - { - name: "stamp 2", - stamp: "2", - expectedDefaultRG: "hcp-underlay-ln-mgmt-2", - expectedResultRG: "hcp-underlay-ln-mgmt-2", - }, - { - name: "stamp 3", - stamp: "3", - expectedDefaultRG: "hcp-underlay-ln-mgmt-3", - expectedResultRG: "hcp-underlay-ln-mgmt-3", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - p, err := configResolver.ValueProvenance(region, tc.stamp, "managementClusterRG") - require.NoError(t, err) - require.Equal(t, tc.expectedDefaultRG, p.Default) - require.Equal(t, tc.expectedResultRG, p.Result) - }) - } -} - func TestMergeConfiguration(t *testing.T) { testCases := []struct { name string diff --git a/config/ev2config/config.go b/config/ev2config/config.go index a6e23233..2cb45c05 100644 --- a/config/ev2config/config.go +++ b/config/ev2config/config.go @@ -11,11 +11,6 @@ import ( _ "embed" ) -const ( - cloudDev = "dev" - cloudPublic = "public" -) - //go:embed config.yaml var rawConfig []byte @@ -49,9 +44,6 @@ func ResolveConfig(cloud, region string) (types.Configuration, error) { } cfg := types.Configuration{} cloudCfg, hasCloud := ev2Config.Clouds[cloud] - if !hasCloud && cloud == cloudDev { - cloudCfg, hasCloud = ev2Config.Clouds[cloudPublic] - } if !hasCloud { return nil, fmt.Errorf("failed to find cloud %s", cloud) } @@ -71,9 +63,6 @@ func ResolveConfigForCloud(cloud string) (types.Configuration, error) { } cloudCfg, hasCloud := ev2Config.Clouds[cloud] - if !hasCloud && cloud == cloudDev { - cloudCfg, hasCloud = ev2Config.Clouds[cloudPublic] - } if !hasCloud { return nil, fmt.Errorf("failed to find cloud %s", cloud) } diff --git a/config/testdata/zz_fixture_TestConfigProvider.yaml b/config/testdata/zz_fixture_TestConfigProvider.yaml index 395f3e88..1a1b396b 100644 --- a/config/testdata/zz_fixture_TestConfigProvider.yaml +++ b/config/testdata/zz_fixture_TestConfigProvider.yaml @@ -39,21 +39,21 @@ imageMirror: adoProject: adoProject artifactName: artifactName buildId: 12345 -imageSyncRG: hcp-underlay-ln-imagesync +imageSyncRG: hcp-underlay-uks-imagesync kusto: cluster: aroINT resourceGroup: aro-kusto-public-int-us serviceLogsDatabase: containerLogs maestro_helm_chart: oci://aro-hcp-int.azurecr.io/helm/server maestro_image: aro-hcp-int.azurecr.io/maestro-server:the-stable-one -managementClusterRG: hcp-underlay-ln-mgmt-1 +managementClusterRG: hcp-underlay-uks-mgmt-1 managementClusterSubscription: hcp-uksouth parentZone: example.com partialValue: public-int-uksouth-value provider: Self region: uksouth -regionRG: hcp-underlay-ln -serviceClusterRG: hcp-underlay-ln-svc +regionRG: hcp-underlay-uks +serviceClusterRG: hcp-underlay-uks-svc serviceClusterSubscription: hcp-uksouth storage: accountName: arotestaccount diff --git a/pipelines/types/pipeline_test.go b/pipelines/types/pipeline_test.go index 4cdae637..057c47aa 100644 --- a/pipelines/types/pipeline_test.go +++ b/pipelines/types/pipeline_test.go @@ -46,7 +46,7 @@ func TestNewPipelineFromFile(t *testing.T) { }) require.NoError(t, err) - cfg, err := resolver.GetRegionConfiguration(region, stamp) + cfg, err := resolver.GetRegionConfiguration(region) assert.NoError(t, err) pipeline, err := NewPipelineFromFile("../testdata/pipeline.yaml", cfg) @@ -222,7 +222,7 @@ resourceGroups: }) require.NoError(t, err) - cfg, err := resolver.GetRegionConfiguration(region, stamp) + cfg, err := resolver.GetRegionConfiguration(region) require.NoError(t, err) _, err = NewPipelineFromBytes([]byte(tt.yaml), cfg) diff --git a/pipelines/types/testdata/zz_fixture_TestNewPipelineFromFile.yaml b/pipelines/types/testdata/zz_fixture_TestNewPipelineFromFile.yaml index 623aafd2..001a65bf 100644 --- a/pipelines/types/testdata/zz_fixture_TestNewPipelineFromFile.yaml +++ b/pipelines/types/testdata/zz_fixture_TestNewPipelineFromFile.yaml @@ -20,7 +20,7 @@ resourceGroups: - uksouth singleton: false name: regional - resourceGroup: hcp-underlay-ln + resourceGroup: hcp-underlay-uks steps: - action: Shell adoArtifacts: