From a521933413b95a0470def3f5e85823de3e9e2d41 Mon Sep 17 00:00:00 2001 From: Yingjie Date: Thu, 27 Aug 2026 17:55:47 +0800 Subject: [PATCH 1/2] feat(alzlibtool): add --library-overwrite-enabled to library commands Only `cache create` exposed an opt-in for AlzLib.Options.AllowOverwrite. The remaining commands constructed the library with NewAlzLib(nil), so a member that intentionally redefines an asset provided by one of its dependencies failed during Alzlib.Init with "already exists in the library", before any check, generation or documentation step ran. Register --library-overwrite-enabled (default false) on `check library`, `generate architecture` and `document library`, and assign it to Options.AllowOverwrite before Init. Fixing only `check library` would leave a member that validates but cannot be generated or documented, which is the worst failure mode for a validation tool. Add doc.AlzlibReadmeMdWithOptions so the document command can pass options through; AlzlibReadmeMd delegates to it with nil options and keeps its signature and error text unchanged. Correct the `cache create` long description and flag help, which claimed that definitions are overwritten. They are not: for policy definitions and policy set definitions the flag only suppresses the duplicate error and the earlier library's version is kept. All four help strings now describe the same limitations, and generate/document/cache emit a warning on stderr when the flag is set so that JSON and Markdown artifacts on stdout stay machine readable. No engine behaviour is changed. alzlib.go, assets and deployment are untouched, so consumers such as terraform-provider-alz are unaffected. Refs Azure/Azure-Landing-Zones#4191 --- cmd/alzlibtool/command/cache/create.go | 25 ++- cmd/alzlibtool/command/check/library.go | 35 +++- cmd/alzlibtool/command/check/library_test.go | 113 ++++++++++++ cmd/alzlibtool/command/document/library.go | 38 +++- .../command/document/library_test.go | 96 ++++++++++ .../command/generate/architecture.go | 32 +++- .../generate/architecture_overwrite_test.go | 166 ++++++++++++++++++ .../dependency/alz_library_metadata.json | 6 + .../dependency/alz_policy_default_values.yml | 7 + .../base.alz_archetype_definition.yaml | 8 + .../dependency-pd.alz_policy_definition.json | 35 ++++ .../shared-pa.alz_policy_assignment.json | 20 +++ .../shared.alz_architecture_definition.yaml | 9 + .../member/alz_library_metadata.json | 11 ++ .../member/alz_policy_default_values.yml | 7 + .../member/base.alz_archetype_definition.yaml | 10 ++ .../member-only-pa.alz_policy_assignment.json | 20 +++ .../member-pd.alz_policy_definition.json | 35 ++++ .../shared-pa.alz_policy_assignment.json | 20 +++ .../shared.alz_architecture_definition.yaml | 9 + .../pdmember/alz_library_metadata.json | 11 ++ .../pdmember/alz_policy_default_values.yml | 7 + .../dependency-pd.alz_policy_definition.json | 35 ++++ .../standalone/alz_library_metadata.json | 6 + internal/doc/doc.go | 14 +- internal/doc/doc_test.go | 16 ++ 26 files changed, 777 insertions(+), 14 deletions(-) create mode 100644 cmd/alzlibtool/command/check/library_test.go create mode 100644 cmd/alzlibtool/command/document/library_test.go create mode 100644 cmd/alzlibtool/command/generate/architecture_overwrite_test.go create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/alz_library_metadata.json create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/alz_policy_default_values.yml create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/base.alz_archetype_definition.yaml create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/dependency-pd.alz_policy_definition.json create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/shared-pa.alz_policy_assignment.json create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/shared.alz_architecture_definition.yaml create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/member/alz_library_metadata.json create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/member/alz_policy_default_values.yml create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/member/base.alz_archetype_definition.yaml create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/member/member-only-pa.alz_policy_assignment.json create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/member/member-pd.alz_policy_definition.json create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/member/shared-pa.alz_policy_assignment.json create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/member/shared.alz_architecture_definition.yaml create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/pdmember/alz_library_metadata.json create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/pdmember/alz_policy_default_values.yml create mode 100644 cmd/alzlibtool/command/testdata/libraryoverwrite/pdmember/dependency-pd.alz_policy_definition.json create mode 100644 cmd/alzlibtool/command/testdata/standalone/alz_library_metadata.json diff --git a/cmd/alzlibtool/command/cache/create.go b/cmd/alzlibtool/command/cache/create.go index eeb9f98..ef9d139 100644 --- a/cmd/alzlibtool/command/cache/create.go +++ b/cmd/alzlibtool/command/cache/create.go @@ -40,9 +40,12 @@ A --library value may be either a local path (e.g. ./mylib) or an ALZ Library re the form @, e.g. platform/alz@2026.01.3. The flag may be specified multiple times to combine several libraries; their dependencies are fetched recursively. -Use --library-overwrite-enabled to allow later libraries to overwrite definitions, policy -assignments, role definitions, archetypes and architectures already provided by earlier -libraries. This is useful when layering a custom library on top of a base ALZ library. +Use --library-overwrite-enabled to apply the current library-overwrite behavior while combining +libraries. Policy assignments, role definitions, archetypes, architectures and policy default +values from later libraries replace those of earlier libraries in full, they are not merged +field by field. Policy definitions and policy set definitions are not replaced: the earlier +library's version is kept and the flag only suppresses the duplicate error. Duplicate archetype +override names remain an error regardless of this flag. Use --from-cache to seed from an existing cache file (requires --library and --architecture). Definitions already present in the seed cache are used directly and not re-fetched from Azure, @@ -88,6 +91,13 @@ to update a cache in-place.`, os.Exit(1) } + if libraryOverwriteEnabled { + // stderr only, so verbose stdout stays scriptable. + cmd.PrintErrln( + "Warning: policy (set) definition redefinitions retain the earlier library's version, " + + "and duplicate archetype override names remain an error.") + } + // Read the seed cache BEFORE opening the output file, because --from-cache // and --output may point to the same path. var seedCache *cache.Cache @@ -264,8 +274,9 @@ func init() { createCmd.Flags(). Bool( "library-overwrite-enabled", false, - "Allow later libraries to overwrite definitions, policy assignments, role definitions, "+ - "archetypes and architectures already provided by earlier libraries. Useful when "+ - "layering a custom library on top of a base ALZ library. Requires --library and "+ - "--architecture.") + "Apply the current library-overwrite behavior while combining libraries. Policy "+ + "assignments, role definitions, archetypes, architectures and policy default values from "+ + "later libraries are replaced in full, not merged field by field. Policy (set) definitions "+ + "keep the earlier library's version; for those the flag only suppresses the duplicate "+ + "error. Requires --library and --architecture.") } diff --git a/cmd/alzlibtool/command/check/library.go b/cmd/alzlibtool/command/check/library.go index 1570ee2..f86ee23 100644 --- a/cmd/alzlibtool/command/check/library.go +++ b/cmd/alzlibtool/command/check/library.go @@ -20,11 +20,35 @@ import ( var libraryCmd = cobra.Command{ Use: "library [flags] dir", Short: "Perform operations on an alzlib library member.", - Long: `Primarily used a a tool to check the validity of a library member.`, - Args: cobra.ExactArgs(1), + Long: `Primarily used a a tool to check the validity of a library member. + +Use --library-overwrite-enabled when this library member intentionally redefines assets +that are also provided by its dependencies. Without it, the check fails during library +initialization with an "already exists in the library" error and no checks are run. + +Assets are replaced in full, they are not merged field by field, so the redefining file +must contain the complete asset. Policy assignments, role definitions, archetypes, +architectures and policy default values are replaced by this member's version. Policy +definitions and policy set definitions are not: for those the flag only suppresses the +duplicate error and the dependency version is kept. Duplicate archetype override names +remain an error regardless of this flag.`, + Args: cobra.ExactArgs(1), Run: func(cmd *cobra.Command, args []string) { az := alzlib.NewAlzLib(nil) + libraryOverwriteEnabled, err := cmd.Flags().GetBool("library-overwrite-enabled") + if err != nil { + cmd.PrintErrf( + "%s could not get library-overwrite-enabled flag: %v\n", + cmd.ErrPrefix(), + err, + ) + os.Exit(1) + } + + // Options are read during Init, so this must be set beforehand. + az.Options.AllowOverwrite = libraryOverwriteEnabled + offline, err := cmd.Flags().GetBool("offline") if err != nil { cmd.PrintErrf("%s could not get offline flag: %v\n", cmd.ErrPrefix(), err) @@ -104,4 +128,11 @@ func init() { "Whether to fix any fixable issues (currently only filename issues).") libraryCmd.Flags(). Bool("offline", false, "Whether to run the checks in offline mode (no Azure calls).") + libraryCmd.Flags(). + Bool( + "library-overwrite-enabled", false, + "Allow this library member to redefine assets provided by its dependencies. Policy "+ + "assignments, role definitions, archetypes, architectures and policy default values are "+ + "replaced in full, not merged field by field. Policy (set) definitions keep the "+ + "dependency version; for those the flag only suppresses the duplicate error.") } diff --git a/cmd/alzlibtool/command/check/library_test.go b/cmd/alzlibtool/command/check/library_test.go new file mode 100644 index 0000000..89e1309 --- /dev/null +++ b/cmd/alzlibtool/command/check/library_test.go @@ -0,0 +1,113 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package check + +import ( + "context" + "fmt" + "os" + "os/exec" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// helperEnvVar, when set, makes the test binary run the library check command instead of the +// tests. This lets the tests observe the os.Exit calls made by the command as an exit code. +const helperEnvVar = "ALZLIBTOOL_CHECK_LIBRARY_HELPER_ARGS" + +// helperArgSeparator separates the command arguments passed via helperEnvVar. +// Environment variables cannot contain NUL, and no argument contains a newline. +const helperArgSeparator = "\n" + +// Fixtures shared with the generate and document command tests. +const ( + overwriteMemberLib = "../testdata/libraryoverwrite/member" + overwritePdMemberLib = "../testdata/libraryoverwrite/pdmember" + standaloneLib = "../testdata/standalone" +) + +func TestMain(m *testing.M) { + if rawArgs, ok := os.LookupEnv(helperEnvVar); ok { + // libraryCmd has CheckCmd as its parent, so the whole tree must be executed. + cmd := CheckCmd + cmd.SetArgs(append([]string{"library"}, strings.Split(rawArgs, helperArgSeparator)...)) + + if err := cmd.ExecuteContext(context.Background()); err != nil { + os.Exit(1) + } + + os.Exit(0) + } + + os.Exit(m.Run()) +} + +// runCheckLibrary runs the library check command in a child process and returns its combined +// output and exit code. +func runCheckLibrary(t *testing.T, args ...string) (string, int) { + t.Helper() + + cmd := exec.Command(os.Args[0]) + + cmd.Env = append( + os.Environ(), + fmt.Sprintf("%s=%s", helperEnvVar, strings.Join(args, helperArgSeparator)), + ) + + out, err := cmd.CombinedOutput() + if err == nil { + return string(out), 0 + } + + var exitErr *exec.ExitError + + require.ErrorAs(t, err, &exitErr, "unexpected error running check library: %v", err) + + return string(out), exitErr.ExitCode() +} + +func Test_libraryCmd_overwriteFlagDefault(t *testing.T) { + f := libraryCmd.Flags().Lookup("library-overwrite-enabled") + require.NotNil(t, f, "library-overwrite-enabled flag should be registered") + assert.Equal(t, "false", f.DefValue) +} + +func Test_libraryCmd_help(t *testing.T) { + out, code := runCheckLibrary(t, "--help") + assert.Equal(t, 0, code) + assert.Contains(t, out, "--library-overwrite-enabled") +} + +func Test_libraryCmd_overwriteDisabledByDefault(t *testing.T) { + out, code := runCheckLibrary(t, overwriteMemberLib, "--offline") + assert.Equal(t, 1, code) + assert.Contains(t, out, "already exists in the library") +} + +// Test_libraryCmd_overwriteEnabled asserts that the member version wins: the member defaults +// require memberParameter, which only exists on the definition referenced by the member version +// of shared-pa, so passing the defaults check proves the dependency version was replaced. +func Test_libraryCmd_overwriteEnabled(t *testing.T) { + out, code := runCheckLibrary(t, overwriteMemberLib, "--offline", "--library-overwrite-enabled") + assert.Equal(t, 0, code, "output: %s", out) + assert.NotContains(t, out, "already exists in the library") +} + +func Test_libraryCmd_noConflictWithoutFlag(t *testing.T) { + out, code := runCheckLibrary(t, standaloneLib, "--offline") + assert.Equal(t, 0, code, "output: %s", out) +} + +// Test_libraryCmd_policyDefinitionOverrideNotApplied pins current engine behaviour: for policy +// (set) definitions the flag only suppresses the duplicate error, the dependency version is +// kept. The pdmember version of dependency-pd adds the parameter its defaults require, so the +// defaults check failing proves the member version was discarded. +func Test_libraryCmd_policyDefinitionOverrideNotApplied(t *testing.T) { + out, code := runCheckLibrary(t, overwritePdMemberLib, "--offline", "--library-overwrite-enabled") + assert.Equal(t, 1, code) + assert.Contains(t, out, "not present in the referenced definition") +} diff --git a/cmd/alzlibtool/command/document/library.go b/cmd/alzlibtool/command/document/library.go index 419921d..ba399ed 100644 --- a/cmd/alzlibtool/command/document/library.go +++ b/cmd/alzlibtool/command/document/library.go @@ -14,8 +14,17 @@ import ( var documentLibraryBaseCmd = cobra.Command{ Use: "library path", Short: "Generates documentation for the supplied library path.", - Long: `Generates documentation for the supplied library path.`, - Args: cobra.ExactArgs(1), + Long: `Generates documentation for the supplied library path. + +Use --library-overwrite-enabled when this library member intentionally redefines assets that +are also provided by its dependencies. Without it, documentation generation fails during +library initialization with an "already exists in the library" error. + +The generated documentation follows the current overwrite semantics: policy assignments, role +definitions, archetypes, architectures and policy default values are replaced in full by this +member's version. Policy definitions and policy set definitions are not, the dependency +version is kept. Duplicate archetype override names remain an error regardless of this flag.`, + Args: cobra.ExactArgs(1), Run: func(cmd *cobra.Command, args []string) { thislib := alzlib.NewCustomLibraryReference(args[0]) @@ -29,10 +38,33 @@ var documentLibraryBaseCmd = cobra.Command{ os.Exit(1) } - err = doc.AlzlibReadmeMd(cmd.Context(), os.Stdout, alllibs...) + libraryOverwriteEnabled, _ := cmd.Flags().GetBool("library-overwrite-enabled") + + opts := alzlib.NewAlzLib(nil).Options + opts.AllowOverwrite = libraryOverwriteEnabled + + if libraryOverwriteEnabled { + // stderr only, so the Markdown on stdout stays usable in a pipeline. + cmd.PrintErrln( + "Warning: generated documentation follows the existing overwrite semantics; " + + "policy (set) definition redefinitions retain the dependency version, and " + + "duplicate archetype override names remain an error.") + } + + err = doc.AlzlibReadmeMdWithOptions(cmd.Context(), os.Stdout, opts, alllibs...) if err != nil { cmd.PrintErrf("%s library documentation error: %v\n", cmd.ErrPrefix(), err) os.Exit(1) } }, } + +func init() { + documentLibraryBaseCmd.Flags(). + Bool( + "library-overwrite-enabled", false, + "Document the library using the current overwrite semantics. Policy assignments, role "+ + "definitions, archetypes, architectures and policy default values are replaced in full, "+ + "not merged field by field. Policy (set) definitions keep the dependency version; for "+ + "those the flag only suppresses the duplicate error.") +} diff --git a/cmd/alzlibtool/command/document/library_test.go b/cmd/alzlibtool/command/document/library_test.go new file mode 100644 index 0000000..6467e9a --- /dev/null +++ b/cmd/alzlibtool/command/document/library_test.go @@ -0,0 +1,96 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package document + +import ( + "context" + "fmt" + "os" + "os/exec" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// helperEnvVar, when set, makes the test binary run the document library command instead of the +// tests. This lets the tests observe the os.Exit calls made by the command as an exit code. +const helperEnvVar = "ALZLIBTOOL_DOCUMENT_LIBRARY_HELPER_ARGS" + +// helperArgSeparator separates the command arguments passed via helperEnvVar. +// Environment variables cannot contain NUL, and no argument contains a newline. +const helperArgSeparator = "\n" + +// overwriteMemberLib is shared with the check and generate command tests. +const overwriteMemberLib = "../testdata/libraryoverwrite/member" + +func TestMain(m *testing.M) { + if rawArgs, ok := os.LookupEnv(helperEnvVar); ok { + // documentLibraryBaseCmd has DocumentBaseCmd as its parent, so the whole tree must run. + cmd := DocumentBaseCmd + cmd.SetArgs(append([]string{"library"}, strings.Split(rawArgs, helperArgSeparator)...)) + + if err := cmd.ExecuteContext(context.Background()); err != nil { + os.Exit(1) + } + + os.Exit(0) + } + + os.Exit(m.Run()) +} + +// runDocumentLibrary runs the command in a child process and returns stdout, stderr and the +// exit code separately, so that the Markdown artifact can be checked independently of warnings. +func runDocumentLibrary(t *testing.T, args ...string) (string, string, int) { + t.Helper() + + cmd := exec.Command(os.Args[0]) + + cmd.Env = append( + os.Environ(), + fmt.Sprintf("%s=%s", helperEnvVar, strings.Join(args, helperArgSeparator)), + ) + + var stdout, stderr strings.Builder + + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + err := cmd.Run() + if err == nil { + return stdout.String(), stderr.String(), 0 + } + + var exitErr *exec.ExitError + + require.ErrorAs(t, err, &exitErr, "unexpected error running document library: %v", err) + + return stdout.String(), stderr.String(), exitErr.ExitCode() +} + +func Test_documentLibraryCmd_overwriteFlagDefault(t *testing.T) { + f := documentLibraryBaseCmd.Flags().Lookup("library-overwrite-enabled") + require.NotNil(t, f, "library-overwrite-enabled flag should be registered") + assert.Equal(t, "false", f.DefValue) +} + +func Test_documentLibraryCmd_overwriteDisabledByDefault(t *testing.T) { + _, stderr, code := runDocumentLibrary(t, overwriteMemberLib) + + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "already exists in the library") +} + +// Test_documentLibraryCmd_overwriteEnabled asserts that the member archetype wins, and that the +// warning goes to stderr so the Markdown on stdout stays usable. +func Test_documentLibraryCmd_overwriteEnabled(t *testing.T) { + stdout, stderr, code := runDocumentLibrary(t, overwriteMemberLib, "--library-overwrite-enabled") + + require.Equal(t, 0, code, "stderr: %s", stderr) + assert.Contains(t, stdout, "member-only-pa") + assert.Contains(t, stderr, "Warning:") + assert.NotContains(t, stdout, "Warning:") +} diff --git a/cmd/alzlibtool/command/generate/architecture.go b/cmd/alzlibtool/command/generate/architecture.go index a7d22f6..bd6bce6 100644 --- a/cmd/alzlibtool/command/generate/architecture.go +++ b/cmd/alzlibtool/command/generate/architecture.go @@ -28,7 +28,17 @@ var generateArchitectureBaseCmd = cobra.Command{ Use: "architecture librarypath name", Short: "Generates deployment JSON for the supplied architecture.", Long: `Generates deployment JSON for the supplied architecture. ` + - `This enables deployment with a tool of your choosing.`, + `This enables deployment with a tool of your choosing. + +Use --library-overwrite-enabled when the library member intentionally redefines assets that +are also provided by its dependencies. Without it, generation fails during library +initialization with an "already exists in the library" error. + +The generated output follows the current overwrite semantics: policy assignments, role +definitions, archetypes, architectures and policy default values are replaced in full by the +member's version. Policy definitions and policy set definitions are not, the dependency +version is kept. Duplicate archetype override names remain an error regardless of this flag. +Verify the generated output before deploying it.`, Args: cobra.ExactArgs(RequiredArchitectureArgs), Run: func(cmd *cobra.Command, args []string) { thisLib := alzlib.NewCustomLibraryReference(args[0]) @@ -45,6 +55,18 @@ var generateArchitectureBaseCmd = cobra.Command{ az := alzlib.NewAlzLib(nil) + libraryOverwriteEnabled, _ := cmd.Flags().GetBool("library-overwrite-enabled") + + // Options are read during Init, so this must be set beforehand. + az.Options.AllowOverwrite = libraryOverwriteEnabled + + if libraryOverwriteEnabled { + // stderr only, so the JSON on stdout stays parseable in a pipeline. + cmd.PrintErrln( + "Warning: policy (set) definition redefinitions retain the dependency version, and " + + "duplicate archetype override names remain an error. Verify the generated output.") + } + // Load seed cache if --from-cache is specified. fromCacheFile, _ := cmd.Flags().GetString("from-cache") if fromCacheFile != "" { @@ -169,4 +191,12 @@ func init() { "Path to a cache file to seed built-in definitions from. "+ "Definitions found in the cache are used before falling back to Azure API calls, "+ "reducing the number of requests made to Azure.") + + generateArchitectureBaseCmd.Flags(). + Bool( + "library-overwrite-enabled", false, + "Apply the current library-overwrite behavior while generating output. Policy "+ + "assignments, role definitions, archetypes, architectures and policy default values are "+ + "replaced in full, not merged field by field. Policy (set) definitions keep the "+ + "dependency version; for those the flag only suppresses the duplicate error.") } diff --git a/cmd/alzlibtool/command/generate/architecture_overwrite_test.go b/cmd/alzlibtool/command/generate/architecture_overwrite_test.go new file mode 100644 index 0000000..4efd77a --- /dev/null +++ b/cmd/alzlibtool/command/generate/architecture_overwrite_test.go @@ -0,0 +1,166 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package generate + +import ( + "context" + "encoding/json" + "fmt" + "io/fs" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// helperEnvVar, when set, makes the test binary run the generate architecture command instead +// of the tests. This lets the tests observe the os.Exit calls made by the command as an exit code. +const helperEnvVar = "ALZLIBTOOL_GENERATE_ARCHITECTURE_HELPER_ARGS" + +// helperArgSeparator separates the command arguments passed via helperEnvVar. +// Environment variables cannot contain NUL, and no argument contains a newline. +const helperArgSeparator = "\n" + +// Fixtures shared with the check and document command tests. Architecture "shared" is provided +// by the dependency and redefined by the member. +const ( + overwriteMemberLib = "../testdata/libraryoverwrite/member" + overwritePdMemberLib = "../testdata/libraryoverwrite/pdmember" + sharedArchitecture = "shared" +) + +func TestMain(m *testing.M) { + if rawArgs, ok := os.LookupEnv(helperEnvVar); ok { + // generateArchitectureBaseCmd has GenerateBaseCmd as its parent, so the tree must run. + cmd := GenerateBaseCmd + cmd.SetArgs(append([]string{"architecture"}, strings.Split(rawArgs, helperArgSeparator)...)) + + if err := cmd.ExecuteContext(context.Background()); err != nil { + os.Exit(1) + } + + os.Exit(0) + } + + os.Exit(m.Run()) +} + +// runGenerateArchitecture runs the command in a child process and returns stdout, stderr and +// the exit code separately, so the JSON artifact can be checked independently of warnings. +func runGenerateArchitecture(t *testing.T, args ...string) (string, string, int) { + t.Helper() + + cmd := exec.Command(os.Args[0]) + + cmd.Env = append( + os.Environ(), + fmt.Sprintf("%s=%s", helperEnvVar, strings.Join(args, helperArgSeparator)), + ) + + var stdout, stderr strings.Builder + + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + err := cmd.Run() + if err == nil { + return stdout.String(), stderr.String(), 0 + } + + var exitErr *exec.ExitError + + require.ErrorAs(t, err, &exitErr, "unexpected error running generate architecture: %v", err) + + return stdout.String(), stderr.String(), exitErr.ExitCode() +} + +func Test_generateArchitectureCmd_overwriteFlagDefault(t *testing.T) { + f := generateArchitectureBaseCmd.Flags().Lookup("library-overwrite-enabled") + require.NotNil(t, f, "library-overwrite-enabled flag should be registered") + assert.Equal(t, "false", f.DefValue) +} + +func Test_generateArchitectureCmd_overwriteDisabledByDefault(t *testing.T) { + _, stderr, code := runGenerateArchitecture(t, overwriteMemberLib, sharedArchitecture) + + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "could not initialize alzlib") + assert.Contains(t, stderr, "already exists in the library") +} + +// Test_generateArchitectureCmd_overwriteEnabled asserts the warning goes to stderr so that the +// JSON on stdout stays parseable in a pipeline. +func Test_generateArchitectureCmd_overwriteEnabled(t *testing.T) { + stdout, stderr, code := runGenerateArchitecture( + t, overwriteMemberLib, sharedArchitecture, "--library-overwrite-enabled", + ) + + require.Equal(t, 0, code, "stderr: %s", stderr) + assert.Contains(t, stderr, "Warning:") + + var parsed []map[string]any + + require.NoError(t, json.Unmarshal([]byte(stdout), &parsed), "stdout must remain valid JSON") + assert.NotEmpty(t, parsed) +} + +// Test_generateArchitectureCmd_outputAssetContract pins what actually lands in the deployment +// artifact: the member policy assignment wins, but a redefined policy definition still comes +// from the dependency, because the engine keeps the dependency version for policy (set) +// definitions. +func Test_generateArchitectureCmd_outputAssetContract(t *testing.T) { + memberOut := t.TempDir() + + _, stderr, code := runGenerateArchitecture( + t, overwriteMemberLib, sharedArchitecture, "--library-overwrite-enabled", "--output", memberOut, + ) + require.Equal(t, 0, code, "stderr: %s", stderr) + assert.Contains(t, readGeneratedAsset(t, memberOut, "shared-pa"), "PA from member") + + pdMemberOut := t.TempDir() + + _, stderr, code = runGenerateArchitecture( + t, overwritePdMemberLib, sharedArchitecture, "--library-overwrite-enabled", "--output", pdMemberOut, + ) + require.Equal(t, 0, code, "stderr: %s", stderr) + + definition := readGeneratedAsset(t, pdMemberOut, "dependency-pd") + + assert.Contains(t, definition, "PD from dependency", + "policy definitions keep the dependency version, see the flag help") + assert.NotContains(t, definition, "PD from pdmember") +} + +func readGeneratedAsset(t *testing.T, dir, name string) string { + t.Helper() + + var found string + + require.NoError(t, filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + if d.IsDir() || !strings.HasPrefix(d.Name(), name+".") { + return nil + } + + b, err := os.ReadFile(path) + if err != nil { + return err + } + + found = string(b) + + return nil + })) + + require.NotEmpty(t, found, "no generated asset found for %q under %s", name, dir) + + return found +} diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/alz_library_metadata.json b/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/alz_library_metadata.json new file mode 100644 index 0000000..142977a --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/alz_library_metadata.json @@ -0,0 +1,6 @@ +{ + "$schema": "https://raw.githubusercontent.com/Azure/Azure-Landing-Zones-Library/main/schemas/library_metadata.json", + "name": "overwritedependency", + "display_name": "overwrite dependency", + "description": "Shared fixture: stands in for an upstream library member that a custom member redefines." +} diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/alz_policy_default_values.yml b/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/alz_policy_default_values.yml new file mode 100644 index 0000000..7dbc528 --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/alz_policy_default_values.yml @@ -0,0 +1,7 @@ +--- +defaults: + - default_name: shared + policy_assignments: + - policy_assignment_name: shared-pa + parameter_names: + - dependencyParameter diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/base.alz_archetype_definition.yaml b/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/base.alz_archetype_definition.yaml new file mode 100644 index 0000000..9960573 --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/base.alz_archetype_definition.yaml @@ -0,0 +1,8 @@ +--- +name: base +policy_assignments: + - shared-pa +policy_definitions: + - dependency-pd +policy_set_definitions: [] +role_definitions: [] diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/dependency-pd.alz_policy_definition.json b/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/dependency-pd.alz_policy_definition.json new file mode 100644 index 0000000..3945b4a --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/dependency-pd.alz_policy_definition.json @@ -0,0 +1,35 @@ +{ + "name": "dependency-pd", + "type": "Microsoft.Authorization/policyDefinitions", + "apiVersion": "2021-06-01", + "scope": null, + "properties": { + "policyType": "Custom", + "mode": "All", + "displayName": "PD from dependency", + "description": "Deliberately declares only dependencyParameter.", + "metadata": { + "version": "1.0.0", + "category": "Test" + }, + "parameters": { + "dependencyParameter": { + "type": "String", + "defaultValue": "Audit", + "metadata": { + "displayName": "Dependency parameter", + "description": "Only present in the dependency version." + } + } + }, + "policyRule": { + "if": { + "field": "type", + "equals": "Microsoft.Resources/subscriptions" + }, + "then": { + "effect": "audit" + } + } + } +} diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/shared-pa.alz_policy_assignment.json b/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/shared-pa.alz_policy_assignment.json new file mode 100644 index 0000000..5e1d4ec --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/shared-pa.alz_policy_assignment.json @@ -0,0 +1,20 @@ +{ + "apiVersion": "2022-06-01", + "dependsOn": [], + "identity": { + "type": "None" + }, + "location": "${default_location}", + "name": "shared-pa", + "properties": { + "description": "PA from dependency", + "displayName": "PA from dependency", + "enforcementMode": null, + "nonComplianceMessages": [], + "notScopes": [], + "parameters": {}, + "policyDefinitionId": "/providers/Microsoft.Authorization/policyDefinitions/dependency-pd", + "scope": "/providers/Microsoft.Management/managementGroups/PLACEHOLDER" + }, + "type": "Microsoft.Authorization/policyAssignments" +} diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/shared.alz_architecture_definition.yaml b/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/shared.alz_architecture_definition.yaml new file mode 100644 index 0000000..aea8682 --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/dependency/shared.alz_architecture_definition.yaml @@ -0,0 +1,9 @@ +--- +name: shared +management_groups: + - archetypes: + - base + display_name: DEPENDENCY + exists: false + id: shared + parent_id: null diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/member/alz_library_metadata.json b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/alz_library_metadata.json new file mode 100644 index 0000000..5beb8a1 --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/alz_library_metadata.json @@ -0,0 +1,11 @@ +{ + "$schema": "https://raw.githubusercontent.com/Azure/Azure-Landing-Zones-Library/main/schemas/library_metadata.json", + "name": "overwritemember", + "display_name": "overwrite member", + "description": "Shared fixture: redefines the assignment, archetype, architecture and defaults of its dependency.", + "dependencies": [ + { + "custom_url": "../testdata/libraryoverwrite/dependency" + } + ] +} diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/member/alz_policy_default_values.yml b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/alz_policy_default_values.yml new file mode 100644 index 0000000..53b1909 --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/alz_policy_default_values.yml @@ -0,0 +1,7 @@ +--- +defaults: + - default_name: shared + policy_assignments: + - policy_assignment_name: shared-pa + parameter_names: + - memberParameter diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/member/base.alz_archetype_definition.yaml b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/base.alz_archetype_definition.yaml new file mode 100644 index 0000000..3b00189 --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/base.alz_archetype_definition.yaml @@ -0,0 +1,10 @@ +--- +name: base +policy_assignments: + - shared-pa + - member-only-pa +policy_definitions: + - dependency-pd + - member-pd +policy_set_definitions: [] +role_definitions: [] diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/member/member-only-pa.alz_policy_assignment.json b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/member-only-pa.alz_policy_assignment.json new file mode 100644 index 0000000..370826d --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/member-only-pa.alz_policy_assignment.json @@ -0,0 +1,20 @@ +{ + "apiVersion": "2022-06-01", + "dependsOn": [], + "identity": { + "type": "None" + }, + "location": "${default_location}", + "name": "member-only-pa", + "properties": { + "description": "Assignment added by the member archetype only.", + "displayName": "Member only assignment", + "enforcementMode": null, + "nonComplianceMessages": [], + "notScopes": [], + "parameters": {}, + "policyDefinitionId": "/providers/Microsoft.Authorization/policyDefinitions/member-pd", + "scope": "/providers/Microsoft.Management/managementGroups/PLACEHOLDER" + }, + "type": "Microsoft.Authorization/policyAssignments" +} diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/member/member-pd.alz_policy_definition.json b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/member-pd.alz_policy_definition.json new file mode 100644 index 0000000..3c526cc --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/member-pd.alz_policy_definition.json @@ -0,0 +1,35 @@ +{ + "name": "member-pd", + "type": "Microsoft.Authorization/policyDefinitions", + "apiVersion": "2021-06-01", + "scope": null, + "properties": { + "policyType": "Custom", + "mode": "All", + "displayName": "PD from member", + "description": "Declares memberParameter, which the member defaults require.", + "metadata": { + "version": "1.0.0", + "category": "Test" + }, + "parameters": { + "memberParameter": { + "type": "String", + "defaultValue": "Audit", + "metadata": { + "displayName": "Member parameter", + "description": "Only present in the member version." + } + } + }, + "policyRule": { + "if": { + "field": "type", + "equals": "Microsoft.Resources/subscriptions" + }, + "then": { + "effect": "deny" + } + } + } +} diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/member/shared-pa.alz_policy_assignment.json b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/shared-pa.alz_policy_assignment.json new file mode 100644 index 0000000..40f5636 --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/shared-pa.alz_policy_assignment.json @@ -0,0 +1,20 @@ +{ + "apiVersion": "2022-06-01", + "dependsOn": [], + "identity": { + "type": "None" + }, + "location": "${default_location}", + "name": "shared-pa", + "properties": { + "description": "PA from member", + "displayName": "PA from member", + "enforcementMode": null, + "nonComplianceMessages": [], + "notScopes": [], + "parameters": {}, + "policyDefinitionId": "/providers/Microsoft.Authorization/policyDefinitions/member-pd", + "scope": "/providers/Microsoft.Management/managementGroups/PLACEHOLDER" + }, + "type": "Microsoft.Authorization/policyAssignments" +} diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/member/shared.alz_architecture_definition.yaml b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/shared.alz_architecture_definition.yaml new file mode 100644 index 0000000..4240528 --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/member/shared.alz_architecture_definition.yaml @@ -0,0 +1,9 @@ +--- +name: shared +management_groups: + - archetypes: + - base + display_name: MEMBER + exists: false + id: shared + parent_id: null diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/pdmember/alz_library_metadata.json b/cmd/alzlibtool/command/testdata/libraryoverwrite/pdmember/alz_library_metadata.json new file mode 100644 index 0000000..cb4dcc6 --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/pdmember/alz_library_metadata.json @@ -0,0 +1,11 @@ +{ + "$schema": "https://raw.githubusercontent.com/Azure/Azure-Landing-Zones-Library/main/schemas/library_metadata.json", + "name": "overwritepdmember", + "display_name": "overwrite policy definition member", + "description": "Shared fixture: redefines only a policy definition, which the engine does not replace.", + "dependencies": [ + { + "custom_url": "../testdata/libraryoverwrite/dependency" + } + ] +} diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/pdmember/alz_policy_default_values.yml b/cmd/alzlibtool/command/testdata/libraryoverwrite/pdmember/alz_policy_default_values.yml new file mode 100644 index 0000000..8b8a974 --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/pdmember/alz_policy_default_values.yml @@ -0,0 +1,7 @@ +--- +defaults: + - default_name: shared + policy_assignments: + - policy_assignment_name: shared-pa + parameter_names: + - pdMemberParameter diff --git a/cmd/alzlibtool/command/testdata/libraryoverwrite/pdmember/dependency-pd.alz_policy_definition.json b/cmd/alzlibtool/command/testdata/libraryoverwrite/pdmember/dependency-pd.alz_policy_definition.json new file mode 100644 index 0000000..5ee9bd2 --- /dev/null +++ b/cmd/alzlibtool/command/testdata/libraryoverwrite/pdmember/dependency-pd.alz_policy_definition.json @@ -0,0 +1,35 @@ +{ + "name": "dependency-pd", + "type": "Microsoft.Authorization/policyDefinitions", + "apiVersion": "2021-06-01", + "scope": null, + "properties": { + "policyType": "Custom", + "mode": "All", + "displayName": "PD from pdmember", + "description": "Adds pdMemberParameter, which the pdmember defaults require.", + "metadata": { + "version": "1.0.0", + "category": "Test" + }, + "parameters": { + "pdMemberParameter": { + "type": "String", + "defaultValue": "Audit", + "metadata": { + "displayName": "Policy definition member parameter", + "description": "Only present in the pdmember version." + } + } + }, + "policyRule": { + "if": { + "field": "type", + "equals": "Microsoft.Resources/subscriptions" + }, + "then": { + "effect": "deny" + } + } + } +} diff --git a/cmd/alzlibtool/command/testdata/standalone/alz_library_metadata.json b/cmd/alzlibtool/command/testdata/standalone/alz_library_metadata.json new file mode 100644 index 0000000..8b56ff5 --- /dev/null +++ b/cmd/alzlibtool/command/testdata/standalone/alz_library_metadata.json @@ -0,0 +1,6 @@ +{ + "$schema": "https://raw.githubusercontent.com/Azure/Azure-Landing-Zones-Library/main/schemas/library_metadata.json", + "name": "standalone", + "display_name": "standalone", + "description": "Conflict-free library used to assert the default (no flag) behaviour is unchanged." +} diff --git a/internal/doc/doc.go b/internal/doc/doc.go index 8839044..715f337 100644 --- a/internal/doc/doc.go +++ b/internal/doc/doc.go @@ -23,7 +23,19 @@ var ( // AlzlibReadmeMd generates a Markdown formatted README for the given alzlib libraries. func AlzlibReadmeMd(ctx context.Context, w io.Writer, libs ...alzlib.LibraryReference) error { - az := alzlib.NewAlzLib(nil) + return AlzlibReadmeMdWithOptions(ctx, w, nil, libs...) +} + +// AlzlibReadmeMdWithOptions is AlzlibReadmeMd with control over the alzlib options, so that +// callers can document a library member that redefines assets provided by its dependencies. +// A nil opts uses the alzlib defaults. +func AlzlibReadmeMdWithOptions( + ctx context.Context, + w io.Writer, + opts *alzlib.Options, + libs ...alzlib.LibraryReference, +) error { + az := alzlib.NewAlzLib(opts) if err := az.Init(ctx, libs...); err != nil { return fmt.Errorf("doc.AlzlibReadmeMd: failed to initialize alzlib: %w", err) } diff --git a/internal/doc/doc_test.go b/internal/doc/doc_test.go index baf56d3..432944d 100644 --- a/internal/doc/doc_test.go +++ b/internal/doc/doc_test.go @@ -24,3 +24,19 @@ func TestAlzlibReadmeMd(t *testing.T) { t.Log(buf.String()) require.NoError(t, err) } + +// TestAlzlibReadmeMdWithOptionsNilMatchesDefault pins that the original entry point keeps its +// exact behaviour now that it delegates to the options-aware one. +func TestAlzlibReadmeMdWithOptionsNilMatchesDefault(t *testing.T) { + ctx := context.Background() + lib := alzlib.NewCustomLibraryReference("../../testdata/simple") + _, err := lib.Fetch(ctx, "simple") + require.NoError(t, err) + + var withDefaults, withNilOptions bytes.Buffer + + require.NoError(t, AlzlibReadmeMd(ctx, &withDefaults, lib)) + require.NoError(t, AlzlibReadmeMdWithOptions(ctx, &withNilOptions, nil, lib)) + + require.Equal(t, withDefaults.String(), withNilOptions.String()) +} From 13793519bd2746bec22ec12775a3f760eef38795 Mon Sep 17 00:00:00 2001 From: Yingjie Date: Tue, 1 Sep 2026 14:31:49 +0800 Subject: [PATCH 2/2] Update typo and error message --- cmd/alzlibtool/command/check/library.go | 2 +- internal/doc/doc.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/alzlibtool/command/check/library.go b/cmd/alzlibtool/command/check/library.go index f86ee23..66d3f84 100644 --- a/cmd/alzlibtool/command/check/library.go +++ b/cmd/alzlibtool/command/check/library.go @@ -20,7 +20,7 @@ import ( var libraryCmd = cobra.Command{ Use: "library [flags] dir", Short: "Perform operations on an alzlib library member.", - Long: `Primarily used a a tool to check the validity of a library member. + Long: `Primarily used as a tool to check the validity of a library member. Use --library-overwrite-enabled when this library member intentionally redefines assets that are also provided by its dependencies. Without it, the check fails during library diff --git a/internal/doc/doc.go b/internal/doc/doc.go index 715f337..8aaa496 100644 --- a/internal/doc/doc.go +++ b/internal/doc/doc.go @@ -37,7 +37,7 @@ func AlzlibReadmeMdWithOptions( ) error { az := alzlib.NewAlzLib(opts) if err := az.Init(ctx, libs...); err != nil { - return fmt.Errorf("doc.AlzlibReadmeMd: failed to initialize alzlib: %w", err) + return fmt.Errorf("doc.AlzlibReadmeMdWithOptions: failed to initialize alzlib: %w", err) } metadataS := az.Metadata()