Prevent circular dependencies between v2 resources and generate a mermaid diagram with their dependencies (#19230)

agent/consul/server.go

@@ -976,7 +976,7 @@ func (s *Server) registerControllers(deps Deps, proxyUpdater ProxyUpdater) error
 		demo.RegisterControllers(s.controllerManager)
 	}
 
-	return nil
+	return s.controllerManager.ValidateDependencies(s.registry.Types())
 }
 
 func newGRPCHandlerFromConfig(deps Deps, config *Config, s *Server) connHandler {

agent/consul/server_test.go

@@ -6,9 +6,11 @@ package consul
 import (
 	"context"
 	"crypto/x509"
+	"flag"
 	"fmt"
 	"net"
 	"os"
+	"path/filepath"
 	"reflect"
 	"strings"
 	"sync"
@@ -35,10 +37,12 @@ import (
 	external "github.com/hashicorp/consul/agent/grpc-external"
 	grpcmiddleware "github.com/hashicorp/consul/agent/grpc-middleware"
 	hcpclient "github.com/hashicorp/consul/agent/hcp/client"
+	"github.com/hashicorp/consul/agent/leafcert"
 	"github.com/hashicorp/consul/agent/metadata"
 	"github.com/hashicorp/consul/agent/rpc/middleware"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/agent/token"
+	proxytracker "github.com/hashicorp/consul/internal/mesh/proxy-tracker"
 	"github.com/hashicorp/consul/ipaddr"
 	"github.com/hashicorp/consul/sdk/freeport"
 	"github.com/hashicorp/consul/sdk/testutil"
@@ -336,7 +340,8 @@ func newServerWithDeps(t *testing.T, c *Config, deps Deps) (*Server, error) {
 		}
 	}
 	grpcServer := external.NewServer(deps.Logger.Named("grpc.external"), nil, deps.TLSConfigurator, rpcRate.NullRequestLimitsHandler())
-	srv, err := NewServer(c, deps, grpcServer, nil, deps.Logger, nil)
+	proxyUpdater := proxytracker.NewProxyTracker(proxytracker.ProxyTrackerConfig{})
+	srv, err := NewServer(c, deps, grpcServer, nil, deps.Logger, proxyUpdater)
 	if err != nil {
 		return nil, err
 	}
@@ -2100,3 +2105,38 @@ func TestServer_hcpManager(t *testing.T) {
 	hcp1.AssertExpectations(t)
 
 }
+
+// goldenMarkdown reads and optionally writes the expected data to the goldenMarkdown file,
+// returning the contents as a string.
+func goldenMarkdown(t *testing.T, name, got string) string {
+	t.Helper()
+
+	golden := filepath.Join("testdata", name+".md")
+	update := flag.Lookup("update").Value.(flag.Getter).Get().(bool)
+	if update && got != "" {
+		err := os.WriteFile(golden, []byte(got), 0644)
+		require.NoError(t, err)
+	}
+
+	expected, err := os.ReadFile(golden)
+	require.NoError(t, err)
+
+	return string(expected)
+}
+
+func TestServer_ControllerDependencies(t *testing.T) {
+	t.Parallel()
+
+	_, conf := testServerConfig(t)
+	deps := newDefaultDeps(t, conf)
+	deps.Experiments = []string{"resource-apis"}
+	deps.LeafCertManager = &leafcert.Manager{}
+
+	s1, err := newServerWithDeps(t, conf, deps)
+	require.NoError(t, err)
+
+	waitForLeaderEstablishment(t, s1)
+	actual := fmt.Sprintf("```mermaid\n%s\n```", s1.controllerManager.CalculateDependencies(s1.registry.Types()).ToMermaid())
+	expected := goldenMarkdown(t, "v2-resource-dependencies", actual)
+	require.Equal(t, expected, actual)
+}

agent/consul/testdata/v2-resource-dependencies.md

@@ -0,0 +1,45 @@
+```mermaid
+flowchart TD
+  auth/v2beta1/computedtrafficpermissions --> auth/v2beta1/trafficpermissions
+  auth/v2beta1/computedtrafficpermissions --> auth/v2beta1/workloadidentity
+  catalog/v2beta1/failoverpolicy --> catalog/v2beta1/service
+  catalog/v2beta1/healthstatus
+  catalog/v2beta1/node --> catalog/v2beta1/healthstatus
+  catalog/v2beta1/service
+  catalog/v2beta1/serviceendpoints --> catalog/v2beta1/service
+  catalog/v2beta1/serviceendpoints --> catalog/v2beta1/workload
+  catalog/v2beta1/workload --> catalog/v2beta1/healthstatus
+  catalog/v2beta1/workload --> catalog/v2beta1/node
+  demo/v1/album
+  demo/v1/artist
+  demo/v1/concept
+  demo/v1/executive
+  demo/v1/recordlabel
+  demo/v2/album
+  demo/v2/artist
+  internal/v1/tombstone
+  mesh/v2beta1/computedexplicitdestinations --> catalog/v2beta1/service
+  mesh/v2beta1/computedexplicitdestinations --> catalog/v2beta1/workload
+  mesh/v2beta1/computedexplicitdestinations --> mesh/v2beta1/computedroutes
+  mesh/v2beta1/computedexplicitdestinations --> mesh/v2beta1/destinations
+  mesh/v2beta1/computedproxyconfiguration --> catalog/v2beta1/workload
+  mesh/v2beta1/computedproxyconfiguration --> mesh/v2beta1/proxyconfiguration
+  mesh/v2beta1/computedroutes --> catalog/v2beta1/failoverpolicy
+  mesh/v2beta1/computedroutes --> catalog/v2beta1/service
+  mesh/v2beta1/computedroutes --> mesh/v2beta1/destinationpolicy
+  mesh/v2beta1/computedroutes --> mesh/v2beta1/grpcroute
+  mesh/v2beta1/computedroutes --> mesh/v2beta1/httproute
+  mesh/v2beta1/computedroutes --> mesh/v2beta1/tcproute
+  mesh/v2beta1/destinationpolicy
+  mesh/v2beta1/destinations
+  mesh/v2beta1/grpcroute
+  mesh/v2beta1/httproute
+  mesh/v2beta1/proxyconfiguration
+  mesh/v2beta1/proxystatetemplate --> auth/v2beta1/computedtrafficpermissions
+  mesh/v2beta1/proxystatetemplate --> catalog/v2beta1/service
+  mesh/v2beta1/proxystatetemplate --> catalog/v2beta1/workload
+  mesh/v2beta1/proxystatetemplate --> mesh/v2beta1/computedexplicitdestinations
+  mesh/v2beta1/proxystatetemplate --> mesh/v2beta1/computedproxyconfiguration
+  mesh/v2beta1/proxystatetemplate --> mesh/v2beta1/computedroutes
+  mesh/v2beta1/tcproute
+```

internal/controller/dependencies.go

@@ -0,0 +1,100 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package controller
+
+import (
+	"fmt"
+	"sort"
+	"strings"
+
+	"github.com/hashicorp/go-multierror"
+
+	"github.com/hashicorp/consul/internal/resource"
+	"github.com/hashicorp/consul/proto-public/pbresource"
+)
+
+func (m *Manager) ValidateDependencies(registrations []resource.Registration) error {
+	deps := m.CalculateDependencies(registrations)
+
+	return deps.validate()
+}
+
+type Dependencies map[string][]string
+
+func (deps Dependencies) validate() error {
+	var merr error
+	seen := make(map[string]map[string]struct{})
+
+	mkErr := func(src, dst string) error {
+		vals := []string{src, dst}
+		sort.Strings(vals)
+		return fmt.Errorf("circular dependency between %q and %q", vals[0], vals[1])
+	}
+
+	for src, dsts := range deps {
+		seenDsts := seen[src]
+		if len(seenDsts) == 0 {
+			seen[src] = make(map[string]struct{})
+		}
+
+		for _, dst := range dsts {
+			if _, ok := seenDsts[dst]; ok {
+				merr = multierror.Append(merr, mkErr(src, dst))
+			}
+
+			if inverseDsts := seen[dst]; len(inverseDsts) > 0 {
+				if _, ok := inverseDsts[src]; ok {
+					merr = multierror.Append(merr, mkErr(src, dst))
+				}
+			}
+			seen[src][dst] = struct{}{}
+		}
+	}
+
+	return merr
+}
+
+func (m *Manager) CalculateDependencies(registrations []resource.Registration) Dependencies {
+	typeToString := func(t *pbresource.Type) string {
+		return strings.ToLower(fmt.Sprintf("%s/%s/%s", t.Group, t.GroupVersion, t.Kind))
+	}
+
+	out := make(map[string][]string)
+	for _, r := range registrations {
+		out[typeToString(r.Type)] = nil
+	}
+
+	for _, c := range m.controllers {
+		watches := make([]string, 0, len(c.watches))
+		for _, w := range c.watches {
+			watches = append(watches, typeToString(w.watchedType))
+		}
+
+		out[typeToString(c.managedType)] = watches
+	}
+
+	return out
+}
+
+func (deps Dependencies) ToMermaid() string {
+	depStrings := make([]string, 0, len(deps))
+
+	for src, dsts := range deps {
+		if len(dsts) == 0 {
+			depStrings = append(depStrings, fmt.Sprintf("  %s", src))
+			continue
+		}
+
+		for _, dst := range dsts {
+			depStrings = append(depStrings, fmt.Sprintf("  %s --> %s", src, dst))
+		}
+	}
+
+	sort.Slice(depStrings, func(a, b int) bool {
+		return depStrings[a] < depStrings[b]
+	})
+	out := "flowchart TD\n" + strings.Join(depStrings, "\n")
+
+	return out
+}

internal/controller/dependencies_test.go

@@ -0,0 +1,66 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package controller
+
+import (
+	"testing"
+
+	"github.com/hashicorp/consul/internal/testing/golden"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDependenciesGolden(t *testing.T) {
+	deps := Dependencies{
+		"t1": []string{"t2", "t3"},
+		"t2": []string{"t4"},
+		"t4": []string{"t1"},
+	}
+	mermaid := deps.ToMermaid()
+	expected := golden.Get(t, mermaid, "dependencies.golden")
+	require.Equal(t, expected, mermaid)
+}
+
+func TestValidateDependencies(t *testing.T) {
+	type testCase struct {
+		dependencies Dependencies
+		expectErr    string
+	}
+
+	run := func(t *testing.T, tc testCase) {
+		err := tc.dependencies.validate()
+		if len(tc.expectErr) > 0 {
+			require.Contains(t, err.Error(), tc.expectErr)
+		} else {
+			require.NoError(t, err)
+		}
+
+	}
+
+	cases := map[string]testCase{
+		"empty": {
+			dependencies: nil,
+		},
+		"no circular dependencies": {
+			dependencies: Dependencies{
+				"t1": []string{"t2", "t3"},
+				"t2": []string{"t3"},
+				"t3": []string{"t4"},
+				"t4": nil,
+			},
+		},
+		"with circular dependency": {
+			dependencies: Dependencies{
+				"t1": []string{"t2", "t3"},
+				"t2": []string{"t1"},
+			},
+			expectErr: `circular dependency between "t1" and "t2"`,
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			run(t, tc)
+		})
+	}
+}

internal/controller/testdata/dependencies.golden

@@ -0,0 +1,5 @@
+flowchart TD
+  t1 --> t2
+  t1 --> t3
+  t2 --> t4
+  t4 --> t1