From c79d710f32b49821d303d7d4ddeac23b1f01f41b Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Mon, 13 Jul 2026 23:19:53 -0500 Subject: [PATCH 1/6] [ci]: Add pure database config adapter Extract namespace database configuration behind an internal provider seam. The normal build uses swsscommon, while the pure build parses database_config.json and rejects unsupported global configurations explicitly. Run canonical packages with the pure build tag, prove CGO-free builds, reject SONiC-only dependency leaks, and remove the sibling checkout from hosted pure tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Dawei Huang --- azure-pipelines.yml | 5 - internal/dbconfig/dbconfig.go | 161 +++++++++++++++ internal/dbconfig/provider_pure.go | 136 ++++++++++++ internal/dbconfig/provider_pure_test.go | 138 +++++++++++++ internal/dbconfig/provider_swss.go | 71 +++++++ .../dbconfig/testdata/database_config.json | 22 ++ pure.mk | 45 ++-- sonic_db_config/db_config.go | 193 ++++-------------- 8 files changed, 601 insertions(+), 170 deletions(-) create mode 100644 internal/dbconfig/dbconfig.go create mode 100644 internal/dbconfig/provider_pure.go create mode 100644 internal/dbconfig/provider_pure_test.go create mode 100644 internal/dbconfig/provider_swss.go create mode 100644 internal/dbconfig/testdata/database_config.json diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 6b7ba4923..4ba8c1659 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -111,10 +111,6 @@ stages: clean: true displayName: 'Checkout code' - - checkout: sonic-mgmt-common - clean: true - displayName: 'Checkout sonic-mgmt-common' - - template: .azure/templates/install-go.yml parameters: version: $(GO_VERSION) @@ -123,7 +119,6 @@ stages: set -euo pipefail export PATH=$PATH:/usr/local/go/bin:$(go env GOPATH)/bin cd sonic-gnmi - go mod tidy make -f pure.mk junit-xml displayName: 'Run pure package tests' diff --git a/internal/dbconfig/dbconfig.go b/internal/dbconfig/dbconfig.go new file mode 100644 index 000000000..957ebb7ca --- /dev/null +++ b/internal/dbconfig/dbconfig.go @@ -0,0 +1,161 @@ +package dbconfig + +import ( + "fmt" + "strconv" +) + +const ( + DefaultNamespace = "" + GlobalConfigFile = "/var/run/redis/sonic-db/database_global.json" + ConfigFile = "/var/run/redis/sonic-db/database_config.json" +) + +type provider interface { + initialize() error + reset() error + namespaces() ([]string, error) + dbList(namespace string) ([]string, error) + dbID(name, namespace string) (int, error) + dbSeparator(name, namespace string) (string, error) + dbSocket(name, namespace string) (string, error) + dbHostname(name, namespace string) (string, error) + dbPort(name, namespace string) (int, error) +} + +var initialized bool + +func Init() (err error) { + defer catchException(&err) + initialized = false + return activeProvider.reset() +} + +func DbInit() (err error) { + defer catchException(&err) + if initialized { + return nil + } + if err := activeProvider.initialize(); err != nil { + return err + } + initialized = true + return nil +} + +func GetDbDefaultNamespace() (string, error) { + return DefaultNamespace, nil +} + +func CheckDbMultiNamespace() (multi bool, err error) { + defer catchException(&err) + namespaces, err := GetDbAllNamespaces() + if err != nil { + return false, err + } + return len(namespaces) > 1, nil +} + +func GetDbNonDefaultNamespaces() (nonDefault []string, err error) { + defer catchException(&err) + namespaces, err := GetDbAllNamespaces() + if err != nil { + return nil, err + } + nonDefault = make([]string, 0, len(namespaces)) + for _, namespace := range namespaces { + if namespace != DefaultNamespace { + nonDefault = append(nonDefault, namespace) + } + } + return nonDefault, nil +} + +func GetDbAllNamespaces() (namespaces []string, err error) { + defer catchException(&err) + if err := DbInit(); err != nil { + return nil, err + } + return activeProvider.namespaces() +} + +func GetDbNamespaceFromTarget(target string) (namespace string, found bool, err error) { + defer catchException(&err) + namespaces, err := GetDbAllNamespaces() + if err != nil { + return "", false, err + } + for _, namespace := range namespaces { + if target == namespace { + return target, true, nil + } + } + return "", false, nil +} + +func GetDbList(namespace string) (databases []string, err error) { + defer catchException(&err) + if err := DbInit(); err != nil { + return nil, err + } + return activeProvider.dbList(namespace) +} + +func GetDbId(name, namespace string) (id int, err error) { + defer catchException(&err) + if err := DbInit(); err != nil { + return -1, err + } + return activeProvider.dbID(name, namespace) +} + +func GetDbSeparator(name, namespace string) (separator string, err error) { + defer catchException(&err) + if err := DbInit(); err != nil { + return "", err + } + return activeProvider.dbSeparator(name, namespace) +} + +func GetDbSock(name, namespace string) (socket string, err error) { + defer catchException(&err) + if err := DbInit(); err != nil { + return "", err + } + return activeProvider.dbSocket(name, namespace) +} + +func GetDbHostName(name, namespace string) (hostname string, err error) { + defer catchException(&err) + if err := DbInit(); err != nil { + return "", err + } + return activeProvider.dbHostname(name, namespace) +} + +func GetDbPort(name, namespace string) (port int, err error) { + defer catchException(&err) + if err := DbInit(); err != nil { + return -1, err + } + return activeProvider.dbPort(name, namespace) +} + +func GetDbTcpAddr(name, namespace string) (address string, err error) { + defer catchException(&err) + hostname, err := GetDbHostName(name, namespace) + if err != nil { + return "", err + } + port, err := GetDbPort(name, namespace) + if err != nil { + return "", err + } + return hostname + ":" + strconv.Itoa(port), nil +} + +func catchException(err *error) { + if recovered := recover(); recovered != nil { + *err = fmt.Errorf("%v", recovered) + } +} diff --git a/internal/dbconfig/provider_pure.go b/internal/dbconfig/provider_pure.go new file mode 100644 index 000000000..2f6305f6b --- /dev/null +++ b/internal/dbconfig/provider_pure.go @@ -0,0 +1,136 @@ +//go:build pure + +package dbconfig + +import ( + "encoding/json" + "fmt" + "os" + "sort" +) + +const defaultDatabaseConfigFile = ConfigFile + +var ( + databaseConfigFile = defaultDatabaseConfigFile + activeProvider provider = &fileProvider{} +) + +type fileProvider struct { + config databaseConfig +} + +type databaseConfig struct { + Instances map[string]instanceConfig `json:"INSTANCES"` + Databases map[string]databaseEntry `json:"DATABASES"` + Includes []json.RawMessage `json:"INCLUDES"` +} + +type instanceConfig struct { + Hostname string `json:"hostname"` + Port int `json:"port"` + UnixSocketPath string `json:"unix_socket_path"` +} + +type databaseEntry struct { + ID int `json:"id"` + Separator string `json:"separator"` + Instance string `json:"instance"` +} + +func (p *fileProvider) initialize() error { + data, err := os.ReadFile(databaseConfigFile) + if err != nil { + return fmt.Errorf("read database configuration: %w", err) + } + if err := json.Unmarshal(data, &p.config); err != nil { + return fmt.Errorf("parse database configuration: %w", err) + } + if len(p.config.Includes) > 0 { + return fmt.Errorf("global database configuration is not supported by the pure provider") + } + return nil +} + +func (p *fileProvider) reset() error { + p.config = databaseConfig{} + return nil +} + +func (p *fileProvider) namespaces() ([]string, error) { + return []string{DefaultNamespace}, nil +} + +func (p *fileProvider) dbList(namespace string) ([]string, error) { + if err := validateNamespace(namespace); err != nil { + return nil, err + } + names := make([]string, 0, len(p.config.Databases)) + for name := range p.config.Databases { + names = append(names, name) + } + sort.Strings(names) + return names, nil +} + +func (p *fileProvider) dbID(name, namespace string) (int, error) { + entry, _, err := p.lookup(name, namespace) + if err != nil { + return -1, err + } + return entry.ID, nil +} + +func (p *fileProvider) dbSeparator(name, namespace string) (string, error) { + entry, _, err := p.lookup(name, namespace) + if err != nil { + return "", err + } + return entry.Separator, nil +} + +func (p *fileProvider) dbSocket(name, namespace string) (string, error) { + _, instance, err := p.lookup(name, namespace) + if err != nil { + return "", err + } + return instance.UnixSocketPath, nil +} + +func (p *fileProvider) dbHostname(name, namespace string) (string, error) { + _, instance, err := p.lookup(name, namespace) + if err != nil { + return "", err + } + return instance.Hostname, nil +} + +func (p *fileProvider) dbPort(name, namespace string) (int, error) { + _, instance, err := p.lookup(name, namespace) + if err != nil { + return -1, err + } + return instance.Port, nil +} + +func (p *fileProvider) lookup(name, namespace string) (databaseEntry, instanceConfig, error) { + if err := validateNamespace(namespace); err != nil { + return databaseEntry{}, instanceConfig{}, err + } + entry, ok := p.config.Databases[name] + if !ok { + return databaseEntry{}, instanceConfig{}, fmt.Errorf("database %q not present in standalone database configuration", name) + } + instance, ok := p.config.Instances[entry.Instance] + if !ok { + return databaseEntry{}, instanceConfig{}, fmt.Errorf("instance %q for database %q not present in standalone database configuration", entry.Instance, name) + } + return entry, instance, nil +} + +func validateNamespace(namespace string) error { + if namespace != DefaultNamespace { + return fmt.Errorf("namespace %q not present in standalone database configuration", namespace) + } + return nil +} diff --git a/internal/dbconfig/provider_pure_test.go b/internal/dbconfig/provider_pure_test.go new file mode 100644 index 000000000..2c8150820 --- /dev/null +++ b/internal/dbconfig/provider_pure_test.go @@ -0,0 +1,138 @@ +//go:build pure + +package dbconfig + +import ( + "path/filepath" + "reflect" + "strings" + "testing" +) + +func TestPureProviderReadsDatabaseConfig(t *testing.T) { + usePureConfig(t, filepath.Join("testdata", "database_config.json")) + + id, err := GetDbId("CONFIG_DB", DefaultNamespace) + if err != nil { + t.Fatalf("GetDbId() error = %v", err) + } + if id != 42 { + t.Errorf("GetDbId() = %d, want 42", id) + } + + separator, err := GetDbSeparator("CONFIG_DB", DefaultNamespace) + if err != nil { + t.Fatalf("GetDbSeparator() error = %v", err) + } + if separator != "~" { + t.Errorf("GetDbSeparator() = %q, want %q", separator, "~") + } + + socket, err := GetDbSock("CONFIG_DB", DefaultNamespace) + if err != nil { + t.Fatalf("GetDbSock() error = %v", err) + } + if socket != "/tmp/standalone-redis.sock" { + t.Errorf("GetDbSock() = %q, want %q", socket, "/tmp/standalone-redis.sock") + } + + address, err := GetDbTcpAddr("CONFIG_DB", DefaultNamespace) + if err != nil { + t.Fatalf("GetDbTcpAddr() error = %v", err) + } + if address != "db.example:6388" { + t.Errorf("GetDbTcpAddr() = %q, want %q", address, "db.example:6388") + } +} + +func TestPureProviderDescribesStandaloneNamespace(t *testing.T) { + usePureConfig(t, filepath.Join("testdata", "database_config.json")) + + namespace, err := GetDbDefaultNamespace() + if err != nil { + t.Fatalf("GetDbDefaultNamespace() error = %v", err) + } + if namespace != DefaultNamespace { + t.Errorf("GetDbDefaultNamespace() = %q, want %q", namespace, DefaultNamespace) + } + + namespaces, err := GetDbAllNamespaces() + if err != nil { + t.Fatalf("GetDbAllNamespaces() error = %v", err) + } + if !reflect.DeepEqual(namespaces, []string{DefaultNamespace}) { + t.Errorf("GetDbAllNamespaces() = %q, want default namespace", namespaces) + } + + nonDefault, err := GetDbNonDefaultNamespaces() + if err != nil { + t.Fatalf("GetDbNonDefaultNamespaces() error = %v", err) + } + if len(nonDefault) != 0 { + t.Errorf("GetDbNonDefaultNamespaces() = %q, want none", nonDefault) + } + + multiNamespace, err := CheckDbMultiNamespace() + if err != nil { + t.Fatalf("CheckDbMultiNamespace() error = %v", err) + } + if multiNamespace { + t.Error("CheckDbMultiNamespace() = true, want false") + } + + databases, err := GetDbList(DefaultNamespace) + if err != nil { + t.Fatalf("GetDbList() error = %v", err) + } + wantDatabases := []string{"CONFIG_DB", "STATE_DB"} + if !reflect.DeepEqual(databases, wantDatabases) { + t.Errorf("GetDbList() = %q, want %q", databases, wantDatabases) + } +} + +func TestPureProviderRejectsUnknownDatabaseAndNamespace(t *testing.T) { + usePureConfig(t, filepath.Join("testdata", "database_config.json")) + + if _, err := GetDbId("UNKNOWN_DB", DefaultNamespace); err == nil || !strings.Contains(err.Error(), `database "UNKNOWN_DB"`) { + t.Errorf("GetDbId() error = %v, want unknown database error", err) + } + if _, err := GetDbId("CONFIG_DB", "asic0"); err == nil || !strings.Contains(err.Error(), `namespace "asic0"`) { + t.Errorf("GetDbId() error = %v, want unsupported namespace error", err) + } + + namespace, found, err := GetDbNamespaceFromTarget(DefaultNamespace) + if err != nil { + t.Fatalf("GetDbNamespaceFromTarget(default) error = %v", err) + } + if !found || namespace != DefaultNamespace { + t.Errorf("GetDbNamespaceFromTarget(default) = %q, %t; want default, true", namespace, found) + } + + namespace, found, err = GetDbNamespaceFromTarget("asic0") + if err != nil { + t.Fatalf("GetDbNamespaceFromTarget(asic0) error = %v", err) + } + if found || namespace != "" { + t.Errorf("GetDbNamespaceFromTarget(asic0) = %q, %t; want empty, false", namespace, found) + } +} + +func TestPureProviderRejectsGlobalConfiguration(t *testing.T) { + usePureConfig(t, filepath.Join("..", "..", "testdata", "database_global.json")) + + err := DbInit() + if err == nil || !strings.Contains(err.Error(), "global database configuration") { + t.Errorf("DbInit() error = %v, want unsupported global configuration error", err) + } +} + +func usePureConfig(t *testing.T, path string) { + t.Helper() + databaseConfigFile = path + t.Cleanup(func() { + databaseConfigFile = defaultDatabaseConfigFile + }) + if err := Init(); err != nil { + t.Fatalf("Init() error = %v", err) + } +} diff --git a/internal/dbconfig/provider_swss.go b/internal/dbconfig/provider_swss.go new file mode 100644 index 000000000..7d143acd0 --- /dev/null +++ b/internal/dbconfig/provider_swss.go @@ -0,0 +1,71 @@ +//go:build !pure + +package dbconfig + +import ( + "os" + + "github.com/sonic-net/sonic-gnmi/swsscommon" +) + +var activeProvider provider = swssProvider{} + +type swssProvider struct{} + +func (swssProvider) initialize() error { + if _, err := os.Stat(GlobalConfigFile); err == nil || os.IsExist(err) { + if !swsscommon.SonicDBConfigIsGlobalInit() { + swsscommon.SonicDBConfigInitializeGlobalConfig() + } + } else if !swsscommon.SonicDBConfigIsInit() { + swsscommon.SonicDBConfigInitialize() + } + return nil +} + +func (swssProvider) reset() error { + swsscommon.SonicDBConfigReset() + return nil +} + +func (swssProvider) namespaces() ([]string, error) { + values := swsscommon.SonicDBConfigGetNamespaces() + defer swsscommon.DeleteVectorString(values) + + namespaces := make([]string, 0, int(values.Size())) + for i := 0; i < int(values.Size()); i++ { + namespaces = append(namespaces, values.Get(i)) + } + return namespaces, nil +} + +func (swssProvider) dbList(string) ([]string, error) { + values := swsscommon.SonicDBConfigGetDbList() + defer swsscommon.DeleteVectorString(values) + + databases := make([]string, 0, int(values.Size())) + for i := 0; i < int(values.Size()); i++ { + databases = append(databases, values.Get(i)) + } + return databases, nil +} + +func (swssProvider) dbID(name, namespace string) (int, error) { + return swsscommon.SonicDBConfigGetDbId(name, namespace), nil +} + +func (swssProvider) dbSeparator(name, namespace string) (string, error) { + return swsscommon.SonicDBConfigGetSeparator(name, namespace), nil +} + +func (swssProvider) dbSocket(name, namespace string) (string, error) { + return swsscommon.SonicDBConfigGetDbSock(name, namespace), nil +} + +func (swssProvider) dbHostname(name, namespace string) (string, error) { + return swsscommon.SonicDBConfigGetDbHostname(name, namespace), nil +} + +func (swssProvider) dbPort(name, namespace string) (int, error) { + return swsscommon.SonicDBConfigGetDbPort(name, namespace), nil +} diff --git a/internal/dbconfig/testdata/database_config.json b/internal/dbconfig/testdata/database_config.json new file mode 100644 index 000000000..e59c93125 --- /dev/null +++ b/internal/dbconfig/testdata/database_config.json @@ -0,0 +1,22 @@ +{ + "INSTANCES": { + "standalone": { + "hostname": "db.example", + "port": 6388, + "unix_socket_path": "/tmp/standalone-redis.sock" + } + }, + "DATABASES": { + "CONFIG_DB": { + "id": 42, + "separator": "~", + "instance": "standalone" + }, + "STATE_DB": { + "id": 43, + "separator": "^", + "instance": "standalone" + } + }, + "VERSION": "1.0" +} diff --git a/pure.mk b/pure.mk index 2af6ed6f8..38e00edb3 100644 --- a/pure.mk +++ b/pure.mk @@ -7,6 +7,10 @@ # Go configuration GO ?= go GOROOT ?= $(shell $(GO) env GOROOT) +PURE_CGO_ENABLED ?= 0 +RACE_CGO_ENABLED ?= 1 +PURE_TAG ?= pure +PURE_GO_FLAGS := -tags=$(PURE_TAG) # Discover every package under the canonical pure roots. This makes purity a # path-based invariant instead of an allowlist that can omit new packages. @@ -64,7 +68,7 @@ vet: @echo "Running go vet on pure packages..." @set -e; for pkg in $(PACKAGES); do \ echo "Vetting $$pkg..."; \ - (cd $$pkg && $(GO) vet .); \ + (cd $$pkg && CGO_ENABLED=$(PURE_CGO_ENABLED) $(GO) vet $(PURE_GO_FLAGS) .); \ done # Test - run all tests with coverage @@ -74,7 +78,7 @@ test: @set -e; for pkg in $(PACKAGES); do \ echo ""; \ echo "=== Testing $$pkg ==="; \ - (cd $$pkg && $(GO) test -gcflags="all=-N -l" -v -race -coverprofile=coverage.out -covermode=atomic .); \ + (cd $$pkg && CGO_ENABLED=$(RACE_CGO_ENABLED) $(GO) test $(PURE_GO_FLAGS) -gcflags="all=-N -l" -v -race -coverprofile=coverage.out -covermode=atomic .); \ if [ -f $$pkg/coverage.out ]; then \ echo "Coverage for $$pkg:"; \ (cd $$pkg && $(GO) tool cover -func=coverage.out); \ @@ -88,7 +92,7 @@ azure-coverage: @set -e; for pkg in $(PACKAGES); do \ echo "Testing $$pkg..."; \ pkgname=$$(echo $$pkg | tr '/' '-'); \ - $(GO) test -gcflags="all=-N -l" -race -coverprofile=coverage-pure-$$pkgname.txt -covermode=atomic -v ./$$pkg; \ + CGO_ENABLED=$(RACE_CGO_ENABLED) $(GO) test $(PURE_GO_FLAGS) -gcflags="all=-N -l" -race -coverprofile=coverage-pure-$$pkgname.txt -covermode=atomic -v ./$$pkg; \ done @echo "Coverage files generated for Azure pipeline" @@ -137,7 +141,7 @@ build-test: @echo "Testing build of pure packages..." @set -e; for pkg in $(PACKAGES); do \ echo "Building $$pkg..."; \ - (cd $$pkg && $(GO) build -v .); \ + (cd $$pkg && CGO_ENABLED=$(PURE_CGO_ENABLED) $(GO) build $(PURE_GO_FLAGS) -v .); \ done # Lint check using basic go tools @@ -151,7 +155,7 @@ bench: @echo "Running benchmarks for pure packages..." @set -e; for pkg in $(PACKAGES); do \ echo "Benchmarking $$pkg..."; \ - (cd $$pkg && $(GO) test -bench=. -benchmem .); \ + (cd $$pkg && CGO_ENABLED=$(PURE_CGO_ENABLED) $(GO) test $(PURE_GO_FLAGS) -bench=. -benchmem .); \ done # Module verification @@ -173,14 +177,30 @@ security: @set -e; if command -v gosec >/dev/null 2>&1; then \ for pkg in $(PACKAGES); do \ echo "Scanning $$pkg..."; \ - (cd $$pkg && gosec .); \ + (cd $$pkg && gosec -tags=$(PURE_TAG) .); \ done; \ else \ echo "gosec not available, skipping security scan"; \ echo "Install with: go install github.com/securecodewarrior/gosec/v2/cmd/gosec@latest"; \ fi -# List vanilla packages +# Verify that canonical packages do not depend on SONiC-only modules. +.PHONY: check-deps +check-deps: + @echo "Checking pure dependency graph..." + @set -e; \ + deps=$$(CGO_ENABLED=$(PURE_CGO_ENABLED) $(GO) list $(PURE_GO_FLAGS) -deps $(addprefix ./,$(PACKAGES))); \ + for forbidden in \ + github.com/sonic-net/sonic-gnmi/swsscommon \ + github.com/Azure/sonic-mgmt-common; do \ + if printf '%s\n' "$$deps" | grep -Fx "$$forbidden" >/dev/null; then \ + echo "Pure package graph includes forbidden dependency: $$forbidden"; \ + exit 1; \ + fi; \ + done + @echo "Pure dependency graph is clean." + +# List pure packages .PHONY: list-packages list-packages: @echo "Pure packages:" @@ -196,7 +216,7 @@ list-packages: # Full CI pipeline .PHONY: ci -ci: clean lint build-test test +ci: clean check-deps lint build-test test @echo "" @echo "=============================================" @echo "✅ Pure CI completed successfully!" @@ -219,7 +239,7 @@ ci: clean lint build-test test # Note: The Azure pipeline now calls gotestsum directly with set -euo pipefail # This target is kept for local testing convenience .PHONY: junit-xml -junit-xml: clean +junit-xml: clean check-deps @echo "Installing gotestsum for JUnit XML generation..." @if ! command -v gotestsum >/dev/null 2>&1; then \ $(GO) install gotest.tools/gotestsum@v1.11.0; \ @@ -234,9 +254,9 @@ junit-xml: clean @echo "Running pure package tests with JUnit XML output..." @mkdir -p test-results @export PATH=$(PATH):$(shell $(GO) env GOPATH)/bin && \ - gotestsum --junitfile test-results/junit-pure.xml \ + CGO_ENABLED=$(RACE_CGO_ENABLED) gotestsum --junitfile test-results/junit-pure.xml \ --format testname \ - -- -gcflags="all=-N -l" -v -race \ + -- $(PURE_GO_FLAGS) -gcflags="all=-N -l" -v -race \ -coverprofile=test-results/coverage-pure.txt \ -covermode=atomic \ $(addprefix ./,$(PACKAGES)) @@ -262,7 +282,7 @@ junit-xml: clean # Quick check for development .PHONY: quick -quick: fmt-check vet build-test +quick: check-deps fmt-check vet build-test @echo "Quick validation complete for pure packages" # Help target @@ -288,6 +308,7 @@ help: @echo " build-test - Test package builds" @echo " bench - Run benchmarks" @echo " security - Run security scan (requires gosec)" + @echo " check-deps - Reject SONiC-only dependencies from pure packages" @echo " mod-verify - Verify go modules" @echo " list-packages - List pure packages" @echo " clean - Clean build artifacts" diff --git a/sonic_db_config/db_config.go b/sonic_db_config/db_config.go index deb7b1de5..06675e9eb 100644 --- a/sonic_db_config/db_config.go +++ b/sonic_db_config/db_config.go @@ -4,15 +4,15 @@ package dbconfig import ( "fmt" + internaldbconfig "github.com/sonic-net/sonic-gnmi/internal/dbconfig" "github.com/sonic-net/sonic-gnmi/swsscommon" - "os" "strconv" ) const ( - SONIC_DB_GLOBAL_CONFIG_FILE string = "/var/run/redis/sonic-db/database_global.json" - SONIC_DB_CONFIG_FILE string = "/var/run/redis/sonic-db/database_config.json" - SONIC_DEFAULT_NAMESPACE string = "" + SONIC_DB_GLOBAL_CONFIG_FILE string = internaldbconfig.GlobalConfigFile + SONIC_DB_CONFIG_FILE string = internaldbconfig.ConfigFile + SONIC_DEFAULT_NAMESPACE string = internaldbconfig.DefaultNamespace SONIC_DEFAULT_CONTAINER string = "" ) @@ -26,185 +26,84 @@ func CatchException(err *error) { } func GetDbDefaultNamespace() (ns string, err error) { - return SONIC_DEFAULT_NAMESPACE, nil + return internaldbconfig.GetDbDefaultNamespace() } func CheckDbMultiNamespace() (ret bool, err error) { - if !sonic_db_init { - err = DbInit() - if err != nil { - return false, err - } + if err := DbInit(); err != nil { + return false, err } - defer CatchException(&err) - ns_vec := swsscommon.SonicDBConfigGetNamespaces() - defer func() { - swsscommon.DeleteVectorString(ns_vec) - }() - length := int(ns_vec.Size()) - // If there are more than one namespaces, this means that SONiC is using multinamespace - return length > 1, err + return internaldbconfig.CheckDbMultiNamespace() } func GetDbNonDefaultNamespaces() (ns_list []string, err error) { - if !sonic_db_init { - err = DbInit() - if err != nil { - return ns_list, err - } + if err := DbInit(); err != nil { + return nil, err } - defer CatchException(&err) - ns_vec := swsscommon.SonicDBConfigGetNamespaces() - defer func() { - swsscommon.DeleteVectorString(ns_vec) - }() - // Translate from vector to array - length := int(ns_vec.Size()) - for i := 0; i < length; i += 1 { - ns := ns_vec.Get(i) - if ns == SONIC_DEFAULT_NAMESPACE { - continue - } - ns_list = append(ns_list, ns) - } - return ns_list, err + return internaldbconfig.GetDbNonDefaultNamespaces() } func GetDbAllNamespaces() (ns_list []string, err error) { - if !sonic_db_init { - err = DbInit() - if err != nil { - return ns_list, err - } + if err := DbInit(); err != nil { + return nil, err } - defer CatchException(&err) - ns_vec := swsscommon.SonicDBConfigGetNamespaces() - defer func() { - swsscommon.DeleteVectorString(ns_vec) - }() - // Translate from vector to array - length := int(ns_vec.Size()) - for i := 0; i < length; i += 1 { - ns := ns_vec.Get(i) - ns_list = append(ns_list, ns) - } - return ns_list, err + return internaldbconfig.GetDbAllNamespaces() } func GetDbNamespaceFromTarget(target string) (ns string, ret bool, err error) { - ns, _ = GetDbDefaultNamespace() - if target == ns { - return target, true, nil - } - ns_list, err := GetDbNonDefaultNamespaces() - if err != nil { + if err := DbInit(); err != nil { return "", false, err } - for _, ns := range ns_list { - if target == ns { - return target, true, nil - } - } - return "", false, nil + return internaldbconfig.GetDbNamespaceFromTarget(target) } func GetDbList(ns string) (db_list []string, err error) { - if !sonic_db_init { - err = DbInit() - if err != nil { - return db_list, err - } - } - defer CatchException(&err) - db_vec := swsscommon.SonicDBConfigGetDbList() - defer func() { - swsscommon.DeleteVectorString(db_vec) - }() - // Translate from vector to array - length := int(db_vec.Size()) - for i := 0; i < length; i += 1 { - ns := db_vec.Get(i) - db_list = append(db_list, ns) + if err := DbInit(); err != nil { + return nil, err } - return db_list, err + return internaldbconfig.GetDbList(ns) } func GetDbSeparator(db_name string, ns string) (separator string, err error) { - if !sonic_db_init { - err = DbInit() - if err != nil { - return "", err - } + if err := DbInit(); err != nil { + return "", err } - defer CatchException(&err) - separator = swsscommon.SonicDBConfigGetSeparator(db_name, ns) - return separator, err + return internaldbconfig.GetDbSeparator(db_name, ns) } func GetDbId(db_name string, ns string) (id int, err error) { - if !sonic_db_init { - err = DbInit() - if err != nil { - return -1, err - } + if err := DbInit(); err != nil { + return -1, err } - defer CatchException(&err) - id = swsscommon.SonicDBConfigGetDbId(db_name, ns) - return id, err + return internaldbconfig.GetDbId(db_name, ns) } func GetDbSock(db_name string, ns string) (unix_socket_path string, err error) { - if !sonic_db_init { - err = DbInit() - if err != nil { - return "", err - } + if err := DbInit(); err != nil { + return "", err } - defer CatchException(&err) - unix_socket_path = swsscommon.SonicDBConfigGetDbSock(db_name, ns) - return unix_socket_path, err + return internaldbconfig.GetDbSock(db_name, ns) } func GetDbHostName(db_name string, ns string) (hostname string, err error) { - if !sonic_db_init { - err = DbInit() - if err != nil { - return "", err - } + if err := DbInit(); err != nil { + return "", err } - defer CatchException(&err) - hostname = swsscommon.SonicDBConfigGetDbHostname(db_name, ns) - return hostname, err + return internaldbconfig.GetDbHostName(db_name, ns) } func GetDbPort(db_name string, ns string) (port int, err error) { - if !sonic_db_init { - err = DbInit() - if err != nil { - return -1, err - } + if err := DbInit(); err != nil { + return -1, err } - defer CatchException(&err) - port = swsscommon.SonicDBConfigGetDbPort(db_name, ns) - return port, err + return internaldbconfig.GetDbPort(db_name, ns) } func GetDbTcpAddr(db_name string, ns string) (addr string, err error) { - if !sonic_db_init { - err = DbInit() - if err != nil { - return "", err - } - } - hostname, err := GetDbHostName(db_name, ns) - if err != nil { - return "", err - } - port, err := GetDbPort(db_name, ns) - if err != nil { + if err := DbInit(); err != nil { return "", err } - return hostname + ":" + strconv.Itoa(port), err + return internaldbconfig.GetDbTcpAddr(db_name, ns) } func CheckDbMultiInstance() (ret bool, err error) { @@ -389,26 +288,14 @@ func DbInit() (err error) { if sonic_db_init { return nil } - defer CatchException(&err) - if _, ierr := os.Stat(SONIC_DB_GLOBAL_CONFIG_FILE); ierr == nil || os.IsExist(ierr) { - // If there's global config file, invoke SonicDBConfigInitializeGlobalConfig - if !swsscommon.SonicDBConfigIsGlobalInit() { - swsscommon.SonicDBConfigInitializeGlobalConfig() - } - } else { - // If there's no global config file, invoke SonicDBConfigInitialize - if !swsscommon.SonicDBConfigIsInit() { - swsscommon.SonicDBConfigInitialize() - } + if err := internaldbconfig.DbInit(); err != nil { + return err } sonic_db_init = true - return err + return nil } func Init() (err error) { sonic_db_init = false - defer CatchException(&err) - // Clear database configuration - swsscommon.SonicDBConfigReset() - return err + return internaldbconfig.Init() } From 674034c86d10c373463f053c7afa2ce6b5e73df0 Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Mon, 13 Jul 2026 23:58:59 -0500 Subject: [PATCH 2/6] [ci]: Enforce pure adapter boundaries Validate test dependencies and build-tag placement, run a CGO-free build before hosted race tests, and preserve default namespace lookup behavior. Share the database configuration contract with the real swsscommon provider and publish its integration result separately. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Dawei Huang --- Makefile | 15 ++++- azure-pipelines.yml | 8 +-- internal/dbconfig/dbconfig.go | 3 + internal/dbconfig/provider_contract_test.go | 75 +++++++++++++++++++++ internal/dbconfig/provider_pure.go | 13 +++- internal/dbconfig/provider_pure_test.go | 57 +++++++--------- internal/dbconfig/provider_swss_test.go | 19 ++++++ pure.mk | 38 ++++++++--- sonic_db_config/db_config.go | 3 + 9 files changed, 183 insertions(+), 48 deletions(-) create mode 100644 internal/dbconfig/provider_contract_test.go create mode 100644 internal/dbconfig/provider_swss_test.go diff --git a/Makefile b/Makefile index 4daa82c71..28ae33090 100644 --- a/Makefile +++ b/Makefile @@ -233,6 +233,7 @@ $(ENVFILE): check_gotest: $(DBCONFG) $(ENVFILE) sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race $(TEST_FLAGS) -coverprofile=coverage-telemetry.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/telemetry + sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race $(TEST_FLAGS) -coverprofile=coverage-internal-dbconfig.txt -covermode=atomic -v github.com/sonic-net/sonic-gnmi/internal/dbconfig sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race $(TEST_FLAGS) -coverprofile=coverage-config.txt -covermode=atomic -v github.com/sonic-net/sonic-gnmi/sonic_db_config sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -race -timeout 40m $(TEST_FLAGS) -coverprofile=coverage-gnmi.txt -covermode=atomic -mod=vendor $(BLD_FLAGS) -v github.com/sonic-net/sonic-gnmi/gnmi_server -coverpkg ../... sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -race -timeout 40m $(TEST_FLAGS) -coverprofile=coverage-pathz_authorizer.txt -covermode=atomic -mod=vendor $(BLD_FLAGS) -v github.com/sonic-net/sonic-gnmi/pathz_authorizer -coverpkg ../... @@ -265,6 +266,9 @@ endif # Integration test packages - basic ones (no special environment needed) +INTEGRATION_DB_CONFIG_PKG := \ + github.com/sonic-net/sonic-gnmi/internal/dbconfig + INTEGRATION_BASIC_PKGS := \ github.com/sonic-net/sonic-gnmi/sonic_db_config \ github.com/sonic-net/sonic-gnmi/sonic_service_client \ @@ -329,6 +333,14 @@ check_gotest_junit: $(DBCONFG) $(ENVFILE) sudo $(GO) install gotest.tools/gotestsum@v1.11.0 @echo "Running integration tests with JUnit XML output..." @mkdir -p test-results + + # Run the real dbconfig provider contract separately because it shares + # process-global swsscommon configuration with sonic_db_config tests. + CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" \ + sudo -E $(shell sudo $(GO) env GOPATH)/bin/gotestsum --junitfile test-results/junit-integration-dbconfig.xml \ + --format testname \ + -- -race $(TEST_FLAGS) -coverprofile=test-results/coverage-integration-dbconfig.txt \ + -covermode=atomic -mod=vendor -v $(INTEGRATION_DB_CONFIG_PKG) # TODO: Fix tests to not depend on /etc/sonic existing # Creating directory here as workaround for poorly written tests @@ -394,6 +406,7 @@ endif @echo "Integration JUnit XML generation completed!" @echo "=============================================" @echo "Files generated:" + @echo " - test-results/junit-integration-dbconfig.xml (Database configuration provider)" @echo " - test-results/junit-integration-basic.xml (Basic packages)" @echo " - test-results/junit-integration-env.xml (Environment-dependent packages)" ifneq ($(ENABLE_DIALOUT_VALUE),0) @@ -469,5 +482,3 @@ diff-cover: coverage.xml test-results/coverage-pure.xml --compare-branch $(TARGET_BRANCH) \ --src-roots . \ --fail-under $(DIFF_COVER_THRESHOLD) - - diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 4ba8c1659..494baf2ee 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -118,7 +118,6 @@ stages: - bash: | set -euo pipefail export PATH=$PATH:/usr/local/go/bin:$(go env GOPATH)/bin - cd sonic-gnmi make -f pure.mk junit-xml displayName: 'Run pure package tests' @@ -127,7 +126,7 @@ stages: condition: always() inputs: testResultsFormat: 'JUnit' - testResultsFiles: '$(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/junit-pure.xml' + testResultsFiles: '$(Build.SourcesDirectory)/test-results/junit-pure.xml' failTaskOnFailedTests: true publishRunAttachments: true testRunTitle: 'Pure package tests' @@ -136,9 +135,9 @@ stages: displayName: 'Publish pure package coverage' condition: always() inputs: - summaryFileLocation: '$(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/coverage-pure.xml' + summaryFileLocation: '$(Build.SourcesDirectory)/test-results/coverage-pure.xml' - - publish: $(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/coverage-pure.xml + - publish: $(Build.SourcesDirectory)/test-results/coverage-pure.xml artifact: coverage-pure displayName: 'Publish pure coverage artifact' condition: always() @@ -208,6 +207,7 @@ stages: inputs: testResultsFormat: 'JUnit' testResultsFiles: | + $(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/junit-integration-dbconfig.xml $(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/junit-integration-basic.xml $(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/junit-integration-env.xml $(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/junit-integration-dialout.xml diff --git a/internal/dbconfig/dbconfig.go b/internal/dbconfig/dbconfig.go index 957ebb7ca..e51107e23 100644 --- a/internal/dbconfig/dbconfig.go +++ b/internal/dbconfig/dbconfig.go @@ -81,6 +81,9 @@ func GetDbAllNamespaces() (namespaces []string, err error) { func GetDbNamespaceFromTarget(target string) (namespace string, found bool, err error) { defer catchException(&err) + if target == DefaultNamespace { + return target, true, nil + } namespaces, err := GetDbAllNamespaces() if err != nil { return "", false, err diff --git a/internal/dbconfig/provider_contract_test.go b/internal/dbconfig/provider_contract_test.go new file mode 100644 index 000000000..3d638f3b2 --- /dev/null +++ b/internal/dbconfig/provider_contract_test.go @@ -0,0 +1,75 @@ +package dbconfig + +import ( + "slices" + "testing" +) + +type providerContract struct { + database string + namespace string + id int + separator string + socket string + address string +} + +func runProviderContract(t *testing.T, contract providerContract) { + t.Helper() + + namespace, err := GetDbDefaultNamespace() + if err != nil { + t.Fatalf("GetDbDefaultNamespace() error = %v", err) + } + if namespace != contract.namespace { + t.Errorf("GetDbDefaultNamespace() = %q, want %q", namespace, contract.namespace) + } + + namespaces, err := GetDbAllNamespaces() + if err != nil { + t.Fatalf("GetDbAllNamespaces() error = %v", err) + } + if !slices.Contains(namespaces, contract.namespace) { + t.Errorf("GetDbAllNamespaces() = %q, want namespace %q", namespaces, contract.namespace) + } + + databases, err := GetDbList(contract.namespace) + if err != nil { + t.Fatalf("GetDbList() error = %v", err) + } + if !slices.Contains(databases, contract.database) { + t.Errorf("GetDbList() = %q, want database %q", databases, contract.database) + } + + id, err := GetDbId(contract.database, contract.namespace) + if err != nil { + t.Fatalf("GetDbId() error = %v", err) + } + if id != contract.id { + t.Errorf("GetDbId() = %d, want %d", id, contract.id) + } + + separator, err := GetDbSeparator(contract.database, contract.namespace) + if err != nil { + t.Fatalf("GetDbSeparator() error = %v", err) + } + if separator != contract.separator { + t.Errorf("GetDbSeparator() = %q, want %q", separator, contract.separator) + } + + socket, err := GetDbSock(contract.database, contract.namespace) + if err != nil { + t.Fatalf("GetDbSock() error = %v", err) + } + if socket != contract.socket { + t.Errorf("GetDbSock() = %q, want %q", socket, contract.socket) + } + + address, err := GetDbTcpAddr(contract.database, contract.namespace) + if err != nil { + t.Fatalf("GetDbTcpAddr() error = %v", err) + } + if address != contract.address { + t.Errorf("GetDbTcpAddr() = %q, want %q", address, contract.address) + } +} diff --git a/internal/dbconfig/provider_pure.go b/internal/dbconfig/provider_pure.go index 2f6305f6b..1a0c605bc 100644 --- a/internal/dbconfig/provider_pure.go +++ b/internal/dbconfig/provider_pure.go @@ -4,15 +4,20 @@ package dbconfig import ( "encoding/json" + "errors" "fmt" "os" "sort" ) -const defaultDatabaseConfigFile = ConfigFile +const ( + defaultDatabaseConfigFile = ConfigFile + defaultGlobalConfigFile = GlobalConfigFile +) var ( databaseConfigFile = defaultDatabaseConfigFile + globalConfigFile = defaultGlobalConfigFile activeProvider provider = &fileProvider{} ) @@ -39,6 +44,12 @@ type databaseEntry struct { } func (p *fileProvider) initialize() error { + if _, err := os.Stat(globalConfigFile); err == nil { + return fmt.Errorf("global database configuration is not supported by the pure provider") + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("inspect global database configuration: %w", err) + } + data, err := os.ReadFile(databaseConfigFile) if err != nil { return fmt.Errorf("read database configuration: %w", err) diff --git a/internal/dbconfig/provider_pure_test.go b/internal/dbconfig/provider_pure_test.go index 2c8150820..355136576 100644 --- a/internal/dbconfig/provider_pure_test.go +++ b/internal/dbconfig/provider_pure_test.go @@ -11,38 +11,14 @@ import ( func TestPureProviderReadsDatabaseConfig(t *testing.T) { usePureConfig(t, filepath.Join("testdata", "database_config.json")) - - id, err := GetDbId("CONFIG_DB", DefaultNamespace) - if err != nil { - t.Fatalf("GetDbId() error = %v", err) - } - if id != 42 { - t.Errorf("GetDbId() = %d, want 42", id) - } - - separator, err := GetDbSeparator("CONFIG_DB", DefaultNamespace) - if err != nil { - t.Fatalf("GetDbSeparator() error = %v", err) - } - if separator != "~" { - t.Errorf("GetDbSeparator() = %q, want %q", separator, "~") - } - - socket, err := GetDbSock("CONFIG_DB", DefaultNamespace) - if err != nil { - t.Fatalf("GetDbSock() error = %v", err) - } - if socket != "/tmp/standalone-redis.sock" { - t.Errorf("GetDbSock() = %q, want %q", socket, "/tmp/standalone-redis.sock") - } - - address, err := GetDbTcpAddr("CONFIG_DB", DefaultNamespace) - if err != nil { - t.Fatalf("GetDbTcpAddr() error = %v", err) - } - if address != "db.example:6388" { - t.Errorf("GetDbTcpAddr() = %q, want %q", address, "db.example:6388") - } + runProviderContract(t, providerContract{ + database: "CONFIG_DB", + namespace: DefaultNamespace, + id: 42, + separator: "~", + socket: "/tmp/standalone-redis.sock", + address: "db.example:6388", + }) } func TestPureProviderDescribesStandaloneNamespace(t *testing.T) { @@ -118,7 +94,8 @@ func TestPureProviderRejectsUnknownDatabaseAndNamespace(t *testing.T) { } func TestPureProviderRejectsGlobalConfiguration(t *testing.T) { - usePureConfig(t, filepath.Join("..", "..", "testdata", "database_global.json")) + usePureConfig(t, filepath.Join("testdata", "database_config.json")) + globalConfigFile = filepath.Join("..", "..", "testdata", "database_global.json") err := DbInit() if err == nil || !strings.Contains(err.Error(), "global database configuration") { @@ -126,11 +103,25 @@ func TestPureProviderRejectsGlobalConfiguration(t *testing.T) { } } +func TestDefaultTargetDoesNotInitializeProvider(t *testing.T) { + usePureConfig(t, filepath.Join(t.TempDir(), "missing-database-config.json")) + + namespace, found, err := GetDbNamespaceFromTarget(DefaultNamespace) + if err != nil { + t.Fatalf("GetDbNamespaceFromTarget(default) error = %v", err) + } + if !found || namespace != DefaultNamespace { + t.Errorf("GetDbNamespaceFromTarget(default) = %q, %t; want default, true", namespace, found) + } +} + func usePureConfig(t *testing.T, path string) { t.Helper() databaseConfigFile = path + globalConfigFile = filepath.Join(t.TempDir(), "missing-global-config.json") t.Cleanup(func() { databaseConfigFile = defaultDatabaseConfigFile + globalConfigFile = defaultGlobalConfigFile }) if err := Init(); err != nil { t.Fatalf("Init() error = %v", err) diff --git a/internal/dbconfig/provider_swss_test.go b/internal/dbconfig/provider_swss_test.go new file mode 100644 index 000000000..cff5a6eb3 --- /dev/null +++ b/internal/dbconfig/provider_swss_test.go @@ -0,0 +1,19 @@ +//go:build !pure + +package dbconfig + +import "testing" + +func TestSwssProviderContract(t *testing.T) { + if err := Init(); err != nil { + t.Fatalf("Init() error = %v", err) + } + runProviderContract(t, providerContract{ + database: "CONFIG_DB", + namespace: DefaultNamespace, + id: 4, + separator: "|", + socket: "/var/run/redis/redis.sock", + address: "127.0.0.1:6379", + }) +} diff --git a/pure.mk b/pure.mk index 38e00edb3..96b0c7799 100644 --- a/pure.mk +++ b/pure.mk @@ -189,17 +189,38 @@ security: check-deps: @echo "Checking pure dependency graph..." @set -e; \ - deps=$$(CGO_ENABLED=$(PURE_CGO_ENABLED) $(GO) list $(PURE_GO_FLAGS) -deps $(addprefix ./,$(PACKAGES))); \ + deps=$$(CGO_ENABLED=$(PURE_CGO_ENABLED) $(GO) list $(PURE_GO_FLAGS) -test -deps $(addprefix ./,$(PACKAGES))); \ + for dep in $$deps; do \ for forbidden in \ github.com/sonic-net/sonic-gnmi/swsscommon \ github.com/Azure/sonic-mgmt-common; do \ - if printf '%s\n' "$$deps" | grep -Fx "$$forbidden" >/dev/null; then \ - echo "Pure package graph includes forbidden dependency: $$forbidden"; \ - exit 1; \ - fi; \ + case "$$dep" in \ + "$$forbidden"|"$$forbidden"/*) \ + echo "Pure package graph includes forbidden dependency: $$dep"; \ + exit 1; \ + ;; \ + esac; \ + done; \ done @echo "Pure dependency graph is clean." +# Keep pure build constraints on provider adapters rather than business logic. +.PHONY: check-tags +check-tags: + @echo "Checking pure build-tag placement..." + @set -e; \ + files=$$(grep -R -l '^//go:build .*pure' $(PURE_ROOTS) --include='*.go' 2>/dev/null || true); \ + for file in $$files; do \ + case "$$file" in \ + internal/*/provider_*.go) ;; \ + *) \ + echo "Pure build tag is outside an internal provider adapter: $$file"; \ + exit 1; \ + ;; \ + esac; \ + done + @echo "Pure build-tag placement is clean." + # List pure packages .PHONY: list-packages list-packages: @@ -216,7 +237,7 @@ list-packages: # Full CI pipeline .PHONY: ci -ci: clean check-deps lint build-test test +ci: clean check-deps check-tags lint build-test test @echo "" @echo "=============================================" @echo "✅ Pure CI completed successfully!" @@ -239,7 +260,7 @@ ci: clean check-deps lint build-test test # Note: The Azure pipeline now calls gotestsum directly with set -euo pipefail # This target is kept for local testing convenience .PHONY: junit-xml -junit-xml: clean check-deps +junit-xml: clean check-deps check-tags build-test @echo "Installing gotestsum for JUnit XML generation..." @if ! command -v gotestsum >/dev/null 2>&1; then \ $(GO) install gotest.tools/gotestsum@v1.11.0; \ @@ -282,7 +303,7 @@ junit-xml: clean check-deps # Quick check for development .PHONY: quick -quick: check-deps fmt-check vet build-test +quick: check-deps check-tags fmt-check vet build-test @echo "Quick validation complete for pure packages" # Help target @@ -309,6 +330,7 @@ help: @echo " bench - Run benchmarks" @echo " security - Run security scan (requires gosec)" @echo " check-deps - Reject SONiC-only dependencies from pure packages" + @echo " check-tags - Restrict pure tags to provider adapters" @echo " mod-verify - Verify go modules" @echo " list-packages - List pure packages" @echo " clean - Clean build artifacts" diff --git a/sonic_db_config/db_config.go b/sonic_db_config/db_config.go index 06675e9eb..6af65c3a6 100644 --- a/sonic_db_config/db_config.go +++ b/sonic_db_config/db_config.go @@ -51,6 +51,9 @@ func GetDbAllNamespaces() (ns_list []string, err error) { } func GetDbNamespaceFromTarget(target string) (ns string, ret bool, err error) { + if target == SONIC_DEFAULT_NAMESPACE { + return target, true, nil + } if err := DbInit(); err != nil { return "", false, err } From f126639d18c5e18dc05db2bfd309f66b44c91764 Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Wed, 22 Jul 2026 12:02:17 -0500 Subject: [PATCH 3/6] [ci]: Keep standalone validation additive Signed-off-by: Dawei Huang --- Makefile | 15 +---- azure-pipelines.yml | 13 ++-- internal/dbconfig/provider_contract_test.go | 75 --------------------- internal/dbconfig/provider_pure_test.go | 70 +++++++++++++++++++ internal/dbconfig/provider_swss_test.go | 19 ------ pure.mk | 34 ++-------- 6 files changed, 88 insertions(+), 138 deletions(-) delete mode 100644 internal/dbconfig/provider_contract_test.go delete mode 100644 internal/dbconfig/provider_swss_test.go diff --git a/Makefile b/Makefile index 28ae33090..4daa82c71 100644 --- a/Makefile +++ b/Makefile @@ -233,7 +233,6 @@ $(ENVFILE): check_gotest: $(DBCONFG) $(ENVFILE) sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race $(TEST_FLAGS) -coverprofile=coverage-telemetry.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/telemetry - sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race $(TEST_FLAGS) -coverprofile=coverage-internal-dbconfig.txt -covermode=atomic -v github.com/sonic-net/sonic-gnmi/internal/dbconfig sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race $(TEST_FLAGS) -coverprofile=coverage-config.txt -covermode=atomic -v github.com/sonic-net/sonic-gnmi/sonic_db_config sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -race -timeout 40m $(TEST_FLAGS) -coverprofile=coverage-gnmi.txt -covermode=atomic -mod=vendor $(BLD_FLAGS) -v github.com/sonic-net/sonic-gnmi/gnmi_server -coverpkg ../... sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -race -timeout 40m $(TEST_FLAGS) -coverprofile=coverage-pathz_authorizer.txt -covermode=atomic -mod=vendor $(BLD_FLAGS) -v github.com/sonic-net/sonic-gnmi/pathz_authorizer -coverpkg ../... @@ -266,9 +265,6 @@ endif # Integration test packages - basic ones (no special environment needed) -INTEGRATION_DB_CONFIG_PKG := \ - github.com/sonic-net/sonic-gnmi/internal/dbconfig - INTEGRATION_BASIC_PKGS := \ github.com/sonic-net/sonic-gnmi/sonic_db_config \ github.com/sonic-net/sonic-gnmi/sonic_service_client \ @@ -333,14 +329,6 @@ check_gotest_junit: $(DBCONFG) $(ENVFILE) sudo $(GO) install gotest.tools/gotestsum@v1.11.0 @echo "Running integration tests with JUnit XML output..." @mkdir -p test-results - - # Run the real dbconfig provider contract separately because it shares - # process-global swsscommon configuration with sonic_db_config tests. - CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" \ - sudo -E $(shell sudo $(GO) env GOPATH)/bin/gotestsum --junitfile test-results/junit-integration-dbconfig.xml \ - --format testname \ - -- -race $(TEST_FLAGS) -coverprofile=test-results/coverage-integration-dbconfig.txt \ - -covermode=atomic -mod=vendor -v $(INTEGRATION_DB_CONFIG_PKG) # TODO: Fix tests to not depend on /etc/sonic existing # Creating directory here as workaround for poorly written tests @@ -406,7 +394,6 @@ endif @echo "Integration JUnit XML generation completed!" @echo "=============================================" @echo "Files generated:" - @echo " - test-results/junit-integration-dbconfig.xml (Database configuration provider)" @echo " - test-results/junit-integration-basic.xml (Basic packages)" @echo " - test-results/junit-integration-env.xml (Environment-dependent packages)" ifneq ($(ENABLE_DIALOUT_VALUE),0) @@ -482,3 +469,5 @@ diff-cover: coverage.xml test-results/coverage-pure.xml --compare-branch $(TARGET_BRANCH) \ --src-roots . \ --fail-under $(DIFF_COVER_THRESHOLD) + + diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 494baf2ee..6b7ba4923 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -111,6 +111,10 @@ stages: clean: true displayName: 'Checkout code' + - checkout: sonic-mgmt-common + clean: true + displayName: 'Checkout sonic-mgmt-common' + - template: .azure/templates/install-go.yml parameters: version: $(GO_VERSION) @@ -118,6 +122,8 @@ stages: - bash: | set -euo pipefail export PATH=$PATH:/usr/local/go/bin:$(go env GOPATH)/bin + cd sonic-gnmi + go mod tidy make -f pure.mk junit-xml displayName: 'Run pure package tests' @@ -126,7 +132,7 @@ stages: condition: always() inputs: testResultsFormat: 'JUnit' - testResultsFiles: '$(Build.SourcesDirectory)/test-results/junit-pure.xml' + testResultsFiles: '$(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/junit-pure.xml' failTaskOnFailedTests: true publishRunAttachments: true testRunTitle: 'Pure package tests' @@ -135,9 +141,9 @@ stages: displayName: 'Publish pure package coverage' condition: always() inputs: - summaryFileLocation: '$(Build.SourcesDirectory)/test-results/coverage-pure.xml' + summaryFileLocation: '$(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/coverage-pure.xml' - - publish: $(Build.SourcesDirectory)/test-results/coverage-pure.xml + - publish: $(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/coverage-pure.xml artifact: coverage-pure displayName: 'Publish pure coverage artifact' condition: always() @@ -207,7 +213,6 @@ stages: inputs: testResultsFormat: 'JUnit' testResultsFiles: | - $(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/junit-integration-dbconfig.xml $(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/junit-integration-basic.xml $(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/junit-integration-env.xml $(System.DefaultWorkingDirectory)/sonic-gnmi/test-results/junit-integration-dialout.xml diff --git a/internal/dbconfig/provider_contract_test.go b/internal/dbconfig/provider_contract_test.go deleted file mode 100644 index 3d638f3b2..000000000 --- a/internal/dbconfig/provider_contract_test.go +++ /dev/null @@ -1,75 +0,0 @@ -package dbconfig - -import ( - "slices" - "testing" -) - -type providerContract struct { - database string - namespace string - id int - separator string - socket string - address string -} - -func runProviderContract(t *testing.T, contract providerContract) { - t.Helper() - - namespace, err := GetDbDefaultNamespace() - if err != nil { - t.Fatalf("GetDbDefaultNamespace() error = %v", err) - } - if namespace != contract.namespace { - t.Errorf("GetDbDefaultNamespace() = %q, want %q", namespace, contract.namespace) - } - - namespaces, err := GetDbAllNamespaces() - if err != nil { - t.Fatalf("GetDbAllNamespaces() error = %v", err) - } - if !slices.Contains(namespaces, contract.namespace) { - t.Errorf("GetDbAllNamespaces() = %q, want namespace %q", namespaces, contract.namespace) - } - - databases, err := GetDbList(contract.namespace) - if err != nil { - t.Fatalf("GetDbList() error = %v", err) - } - if !slices.Contains(databases, contract.database) { - t.Errorf("GetDbList() = %q, want database %q", databases, contract.database) - } - - id, err := GetDbId(contract.database, contract.namespace) - if err != nil { - t.Fatalf("GetDbId() error = %v", err) - } - if id != contract.id { - t.Errorf("GetDbId() = %d, want %d", id, contract.id) - } - - separator, err := GetDbSeparator(contract.database, contract.namespace) - if err != nil { - t.Fatalf("GetDbSeparator() error = %v", err) - } - if separator != contract.separator { - t.Errorf("GetDbSeparator() = %q, want %q", separator, contract.separator) - } - - socket, err := GetDbSock(contract.database, contract.namespace) - if err != nil { - t.Fatalf("GetDbSock() error = %v", err) - } - if socket != contract.socket { - t.Errorf("GetDbSock() = %q, want %q", socket, contract.socket) - } - - address, err := GetDbTcpAddr(contract.database, contract.namespace) - if err != nil { - t.Fatalf("GetDbTcpAddr() error = %v", err) - } - if address != contract.address { - t.Errorf("GetDbTcpAddr() = %q, want %q", address, contract.address) - } -} diff --git a/internal/dbconfig/provider_pure_test.go b/internal/dbconfig/provider_pure_test.go index 355136576..fd2526b96 100644 --- a/internal/dbconfig/provider_pure_test.go +++ b/internal/dbconfig/provider_pure_test.go @@ -5,6 +5,7 @@ package dbconfig import ( "path/filepath" "reflect" + "slices" "strings" "testing" ) @@ -127,3 +128,72 @@ func usePureConfig(t *testing.T, path string) { t.Fatalf("Init() error = %v", err) } } + +type providerContract struct { + database string + namespace string + id int + separator string + socket string + address string +} + +func runProviderContract(t *testing.T, contract providerContract) { + t.Helper() + + namespace, err := GetDbDefaultNamespace() + if err != nil { + t.Fatalf("GetDbDefaultNamespace() error = %v", err) + } + if namespace != contract.namespace { + t.Errorf("GetDbDefaultNamespace() = %q, want %q", namespace, contract.namespace) + } + + namespaces, err := GetDbAllNamespaces() + if err != nil { + t.Fatalf("GetDbAllNamespaces() error = %v", err) + } + if !slices.Contains(namespaces, contract.namespace) { + t.Errorf("GetDbAllNamespaces() = %q, want namespace %q", namespaces, contract.namespace) + } + + databases, err := GetDbList(contract.namespace) + if err != nil { + t.Fatalf("GetDbList() error = %v", err) + } + if !slices.Contains(databases, contract.database) { + t.Errorf("GetDbList() = %q, want database %q", databases, contract.database) + } + + id, err := GetDbId(contract.database, contract.namespace) + if err != nil { + t.Fatalf("GetDbId() error = %v", err) + } + if id != contract.id { + t.Errorf("GetDbId() = %d, want %d", id, contract.id) + } + + separator, err := GetDbSeparator(contract.database, contract.namespace) + if err != nil { + t.Fatalf("GetDbSeparator() error = %v", err) + } + if separator != contract.separator { + t.Errorf("GetDbSeparator() = %q, want %q", separator, contract.separator) + } + + socket, err := GetDbSock(contract.database, contract.namespace) + if err != nil { + t.Fatalf("GetDbSock() error = %v", err) + } + if socket != contract.socket { + t.Errorf("GetDbSock() = %q, want %q", socket, contract.socket) + } + + address, err := GetDbTcpAddr(contract.database, contract.namespace) + if err != nil { + t.Fatalf("GetDbTcpAddr() error = %v", err) + } + if address != contract.address { + t.Errorf("GetDbTcpAddr() = %q, want %q", address, contract.address) + } +} diff --git a/internal/dbconfig/provider_swss_test.go b/internal/dbconfig/provider_swss_test.go deleted file mode 100644 index cff5a6eb3..000000000 --- a/internal/dbconfig/provider_swss_test.go +++ /dev/null @@ -1,19 +0,0 @@ -//go:build !pure - -package dbconfig - -import "testing" - -func TestSwssProviderContract(t *testing.T) { - if err := Init(); err != nil { - t.Fatalf("Init() error = %v", err) - } - runProviderContract(t, providerContract{ - database: "CONFIG_DB", - namespace: DefaultNamespace, - id: 4, - separator: "|", - socket: "/var/run/redis/redis.sock", - address: "127.0.0.1:6379", - }) -} diff --git a/pure.mk b/pure.mk index 96b0c7799..e97b20d53 100644 --- a/pure.mk +++ b/pure.mk @@ -73,7 +73,7 @@ vet: # Test - run all tests with coverage .PHONY: test -test: +test: build-test @echo "Running tests for pure packages..." @set -e; for pkg in $(PACKAGES); do \ echo ""; \ @@ -87,7 +87,7 @@ test: # Generate coverage files for Azure pipeline integration .PHONY: azure-coverage -azure-coverage: +azure-coverage: build-test @echo "Generating coverage files for Azure pipeline..." @set -e; for pkg in $(PACKAGES); do \ echo "Testing $$pkg..."; \ @@ -138,10 +138,11 @@ coverage-xml: test # Build test - ensure the package builds .PHONY: build-test build-test: - @echo "Testing build of pure packages..." + @echo "Testing build of pure packages and tests..." @set -e; for pkg in $(PACKAGES); do \ echo "Building $$pkg..."; \ (cd $$pkg && CGO_ENABLED=$(PURE_CGO_ENABLED) $(GO) build $(PURE_GO_FLAGS) -v .); \ + (cd $$pkg && CGO_ENABLED=$(PURE_CGO_ENABLED) $(GO) test $(PURE_GO_FLAGS) -c -o /dev/null .); \ done # Lint check using basic go tools @@ -184,26 +185,6 @@ security: echo "Install with: go install github.com/securecodewarrior/gosec/v2/cmd/gosec@latest"; \ fi -# Verify that canonical packages do not depend on SONiC-only modules. -.PHONY: check-deps -check-deps: - @echo "Checking pure dependency graph..." - @set -e; \ - deps=$$(CGO_ENABLED=$(PURE_CGO_ENABLED) $(GO) list $(PURE_GO_FLAGS) -test -deps $(addprefix ./,$(PACKAGES))); \ - for dep in $$deps; do \ - for forbidden in \ - github.com/sonic-net/sonic-gnmi/swsscommon \ - github.com/Azure/sonic-mgmt-common; do \ - case "$$dep" in \ - "$$forbidden"|"$$forbidden"/*) \ - echo "Pure package graph includes forbidden dependency: $$dep"; \ - exit 1; \ - ;; \ - esac; \ - done; \ - done - @echo "Pure dependency graph is clean." - # Keep pure build constraints on provider adapters rather than business logic. .PHONY: check-tags check-tags: @@ -237,7 +218,7 @@ list-packages: # Full CI pipeline .PHONY: ci -ci: clean check-deps check-tags lint build-test test +ci: clean check-tags lint build-test test @echo "" @echo "=============================================" @echo "✅ Pure CI completed successfully!" @@ -260,7 +241,7 @@ ci: clean check-deps check-tags lint build-test test # Note: The Azure pipeline now calls gotestsum directly with set -euo pipefail # This target is kept for local testing convenience .PHONY: junit-xml -junit-xml: clean check-deps check-tags build-test +junit-xml: clean check-tags build-test @echo "Installing gotestsum for JUnit XML generation..." @if ! command -v gotestsum >/dev/null 2>&1; then \ $(GO) install gotest.tools/gotestsum@v1.11.0; \ @@ -303,7 +284,7 @@ junit-xml: clean check-deps check-tags build-test # Quick check for development .PHONY: quick -quick: check-deps check-tags fmt-check vet build-test +quick: check-tags fmt-check vet build-test @echo "Quick validation complete for pure packages" # Help target @@ -329,7 +310,6 @@ help: @echo " build-test - Test package builds" @echo " bench - Run benchmarks" @echo " security - Run security scan (requires gosec)" - @echo " check-deps - Reject SONiC-only dependencies from pure packages" @echo " check-tags - Restrict pure tags to provider adapters" @echo " mod-verify - Verify go modules" @echo " list-packages - List pure packages" From 53ef9e9e33c8b6d519837fe1765ab57a51479b25 Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Wed, 22 Jul 2026 12:28:15 -0500 Subject: [PATCH 4/6] [dbconfig]: Preserve initialization error handling Signed-off-by: Dawei Huang --- sonic_db_config/db_config.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sonic_db_config/db_config.go b/sonic_db_config/db_config.go index 6af65c3a6..4ecb73be4 100644 --- a/sonic_db_config/db_config.go +++ b/sonic_db_config/db_config.go @@ -291,6 +291,7 @@ func DbInit() (err error) { if sonic_db_init { return nil } + defer CatchException(&err) if err := internaldbconfig.DbInit(); err != nil { return err } @@ -300,5 +301,6 @@ func DbInit() (err error) { func Init() (err error) { sonic_db_init = false + defer CatchException(&err) return internaldbconfig.Init() } From d5f65fe514393152a3f59c06db3bda1a082ca978 Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Wed, 22 Jul 2026 15:31:01 -0500 Subject: [PATCH 5/6] [dbconfig]: Group adapters by domain Signed-off-by: Dawei Huang --- internal/{ => adaptors}/dbconfig/dbconfig.go | 0 internal/{ => adaptors}/dbconfig/provider_pure.go | 0 .../{ => adaptors}/dbconfig/provider_pure_test.go | 2 +- internal/{ => adaptors}/dbconfig/provider_swss.go | 0 .../dbconfig/testdata/database_config.json | 0 pure.mk | 13 +++++++------ sonic_db_config/db_config.go | 2 +- 7 files changed, 9 insertions(+), 8 deletions(-) rename internal/{ => adaptors}/dbconfig/dbconfig.go (100%) rename internal/{ => adaptors}/dbconfig/provider_pure.go (100%) rename internal/{ => adaptors}/dbconfig/provider_pure_test.go (98%) rename internal/{ => adaptors}/dbconfig/provider_swss.go (100%) rename internal/{ => adaptors}/dbconfig/testdata/database_config.json (100%) diff --git a/internal/dbconfig/dbconfig.go b/internal/adaptors/dbconfig/dbconfig.go similarity index 100% rename from internal/dbconfig/dbconfig.go rename to internal/adaptors/dbconfig/dbconfig.go diff --git a/internal/dbconfig/provider_pure.go b/internal/adaptors/dbconfig/provider_pure.go similarity index 100% rename from internal/dbconfig/provider_pure.go rename to internal/adaptors/dbconfig/provider_pure.go diff --git a/internal/dbconfig/provider_pure_test.go b/internal/adaptors/dbconfig/provider_pure_test.go similarity index 98% rename from internal/dbconfig/provider_pure_test.go rename to internal/adaptors/dbconfig/provider_pure_test.go index fd2526b96..d5df233c3 100644 --- a/internal/dbconfig/provider_pure_test.go +++ b/internal/adaptors/dbconfig/provider_pure_test.go @@ -96,7 +96,7 @@ func TestPureProviderRejectsUnknownDatabaseAndNamespace(t *testing.T) { func TestPureProviderRejectsGlobalConfiguration(t *testing.T) { usePureConfig(t, filepath.Join("testdata", "database_config.json")) - globalConfigFile = filepath.Join("..", "..", "testdata", "database_global.json") + globalConfigFile = filepath.Join("..", "..", "..", "testdata", "database_global.json") err := DbInit() if err == nil || !strings.Contains(err.Error(), "global database configuration") { diff --git a/internal/dbconfig/provider_swss.go b/internal/adaptors/dbconfig/provider_swss.go similarity index 100% rename from internal/dbconfig/provider_swss.go rename to internal/adaptors/dbconfig/provider_swss.go diff --git a/internal/dbconfig/testdata/database_config.json b/internal/adaptors/dbconfig/testdata/database_config.json similarity index 100% rename from internal/dbconfig/testdata/database_config.json rename to internal/adaptors/dbconfig/testdata/database_config.json diff --git a/pure.mk b/pure.mk index e97b20d53..f6287f07e 100644 --- a/pure.mk +++ b/pure.mk @@ -185,7 +185,7 @@ security: echo "Install with: go install github.com/securecodewarrior/gosec/v2/cmd/gosec@latest"; \ fi -# Keep pure build constraints on provider adapters rather than business logic. +# Keep pure build constraints under internal/adaptors rather than business logic. .PHONY: check-tags check-tags: @echo "Checking pure build-tag placement..." @@ -193,12 +193,13 @@ check-tags: files=$$(grep -R -l '^//go:build .*pure' $(PURE_ROOTS) --include='*.go' 2>/dev/null || true); \ for file in $$files; do \ case "$$file" in \ - internal/*/provider_*.go) ;; \ - *) \ - echo "Pure build tag is outside an internal provider adapter: $$file"; \ - exit 1; \ + internal/adaptors/*/provider_*.go) \ + relative=$${file#internal/adaptors/}; \ + case "$$relative" in */*/*) ;; *) continue ;; esac; \ ;; \ esac; \ + echo "Pure build tag is outside a direct internal adaptor provider: $$file"; \ + exit 1; \ done @echo "Pure build-tag placement is clean." @@ -310,7 +311,7 @@ help: @echo " build-test - Test package builds" @echo " bench - Run benchmarks" @echo " security - Run security scan (requires gosec)" - @echo " check-tags - Restrict pure tags to provider adapters" + @echo " check-tags - Restrict pure tags to internal adaptors" @echo " mod-verify - Verify go modules" @echo " list-packages - List pure packages" @echo " clean - Clean build artifacts" diff --git a/sonic_db_config/db_config.go b/sonic_db_config/db_config.go index 4ecb73be4..c204c2fb9 100644 --- a/sonic_db_config/db_config.go +++ b/sonic_db_config/db_config.go @@ -4,7 +4,7 @@ package dbconfig import ( "fmt" - internaldbconfig "github.com/sonic-net/sonic-gnmi/internal/dbconfig" + internaldbconfig "github.com/sonic-net/sonic-gnmi/internal/adaptors/dbconfig" "github.com/sonic-net/sonic-gnmi/swsscommon" "strconv" ) From cde90db39e3c19b376394769b61dd37311519593 Mon Sep 17 00:00:00 2001 From: Dawei Huang Date: Wed, 22 Jul 2026 18:08:23 -0500 Subject: [PATCH 6/6] [dbconfig]: Harden adapter configuration errors Signed-off-by: Dawei Huang --- internal/adaptors/dbconfig/provider_pure.go | 2 +- .../adaptors/dbconfig/provider_pure_test.go | 24 +++++++++++++++---- internal/adaptors/dbconfig/provider_swss.go | 6 ++++- pure.mk | 2 +- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/internal/adaptors/dbconfig/provider_pure.go b/internal/adaptors/dbconfig/provider_pure.go index 1a0c605bc..a1c3d863f 100644 --- a/internal/adaptors/dbconfig/provider_pure.go +++ b/internal/adaptors/dbconfig/provider_pure.go @@ -58,7 +58,7 @@ func (p *fileProvider) initialize() error { return fmt.Errorf("parse database configuration: %w", err) } if len(p.config.Includes) > 0 { - return fmt.Errorf("global database configuration is not supported by the pure provider") + return fmt.Errorf("database configuration INCLUDES are not supported by the pure provider") } return nil } diff --git a/internal/adaptors/dbconfig/provider_pure_test.go b/internal/adaptors/dbconfig/provider_pure_test.go index d5df233c3..8aaf74360 100644 --- a/internal/adaptors/dbconfig/provider_pure_test.go +++ b/internal/adaptors/dbconfig/provider_pure_test.go @@ -3,6 +3,7 @@ package dbconfig import ( + "os" "path/filepath" "reflect" "slices" @@ -38,7 +39,7 @@ func TestPureProviderDescribesStandaloneNamespace(t *testing.T) { t.Fatalf("GetDbAllNamespaces() error = %v", err) } if !reflect.DeepEqual(namespaces, []string{DefaultNamespace}) { - t.Errorf("GetDbAllNamespaces() = %q, want default namespace", namespaces) + t.Errorf("GetDbAllNamespaces() = %v, want default namespace", namespaces) } nonDefault, err := GetDbNonDefaultNamespaces() @@ -46,7 +47,7 @@ func TestPureProviderDescribesStandaloneNamespace(t *testing.T) { t.Fatalf("GetDbNonDefaultNamespaces() error = %v", err) } if len(nonDefault) != 0 { - t.Errorf("GetDbNonDefaultNamespaces() = %q, want none", nonDefault) + t.Errorf("GetDbNonDefaultNamespaces() = %v, want none", nonDefault) } multiNamespace, err := CheckDbMultiNamespace() @@ -63,7 +64,7 @@ func TestPureProviderDescribesStandaloneNamespace(t *testing.T) { } wantDatabases := []string{"CONFIG_DB", "STATE_DB"} if !reflect.DeepEqual(databases, wantDatabases) { - t.Errorf("GetDbList() = %q, want %q", databases, wantDatabases) + t.Errorf("GetDbList() = %v, want %v", databases, wantDatabases) } } @@ -104,6 +105,19 @@ func TestPureProviderRejectsGlobalConfiguration(t *testing.T) { } } +func TestPureProviderRejectsIncludes(t *testing.T) { + configFile := filepath.Join(t.TempDir(), "database_config.json") + if err := os.WriteFile(configFile, []byte(`{"INCLUDES":["database_config.json"]}`), 0o600); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + usePureConfig(t, configFile) + + err := DbInit() + if err == nil || !strings.Contains(err.Error(), "INCLUDES") { + t.Errorf("DbInit() error = %v, want unsupported INCLUDES error", err) + } +} + func TestDefaultTargetDoesNotInitializeProvider(t *testing.T) { usePureConfig(t, filepath.Join(t.TempDir(), "missing-database-config.json")) @@ -154,7 +168,7 @@ func runProviderContract(t *testing.T, contract providerContract) { t.Fatalf("GetDbAllNamespaces() error = %v", err) } if !slices.Contains(namespaces, contract.namespace) { - t.Errorf("GetDbAllNamespaces() = %q, want namespace %q", namespaces, contract.namespace) + t.Errorf("GetDbAllNamespaces() = %v, want namespace %q", namespaces, contract.namespace) } databases, err := GetDbList(contract.namespace) @@ -162,7 +176,7 @@ func runProviderContract(t *testing.T, contract providerContract) { t.Fatalf("GetDbList() error = %v", err) } if !slices.Contains(databases, contract.database) { - t.Errorf("GetDbList() = %q, want database %q", databases, contract.database) + t.Errorf("GetDbList() = %v, want database %q", databases, contract.database) } id, err := GetDbId(contract.database, contract.namespace) diff --git a/internal/adaptors/dbconfig/provider_swss.go b/internal/adaptors/dbconfig/provider_swss.go index 7d143acd0..9d71a69eb 100644 --- a/internal/adaptors/dbconfig/provider_swss.go +++ b/internal/adaptors/dbconfig/provider_swss.go @@ -3,6 +3,8 @@ package dbconfig import ( + "errors" + "fmt" "os" "github.com/sonic-net/sonic-gnmi/swsscommon" @@ -13,10 +15,12 @@ var activeProvider provider = swssProvider{} type swssProvider struct{} func (swssProvider) initialize() error { - if _, err := os.Stat(GlobalConfigFile); err == nil || os.IsExist(err) { + if _, err := os.Stat(GlobalConfigFile); err == nil { if !swsscommon.SonicDBConfigIsGlobalInit() { swsscommon.SonicDBConfigInitializeGlobalConfig() } + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("inspect global database configuration: %w", err) } else if !swsscommon.SonicDBConfigIsInit() { swsscommon.SonicDBConfigInitialize() } diff --git a/pure.mk b/pure.mk index f6287f07e..4537155bb 100644 --- a/pure.mk +++ b/pure.mk @@ -190,7 +190,7 @@ security: check-tags: @echo "Checking pure build-tag placement..." @set -e; \ - files=$$(grep -R -l '^//go:build .*pure' $(PURE_ROOTS) --include='*.go' 2>/dev/null || true); \ + files=$$(find $(PURE_ROOTS) -type f -name '*.go' -exec grep -l '^//go:build .*pure' {} + 2>/dev/null || true); \ for file in $$files; do \ case "$$file" in \ internal/adaptors/*/provider_*.go) \