-
Notifications
You must be signed in to change notification settings - Fork 10
Base Configs Support - Phase 1 #455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 22 commits
0eb8f89
17d28a8
4c2f544
a6e3ccd
e90f0a7
a80847b
2e8bbdd
2ba2a8f
3e0aad7
e10c7d1
15e0bd6
7a61336
3017e68
ed28023
efc0bf2
deb63e5
6ea0f26
89eaa04
4070cdb
9737ab1
f9dc60b
a46a4b6
b1a4166
7d8022e
344f754
b8895b7
a6ea660
fcbee70
2ba46bf
2bff675
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package imagecustomizerapi | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "strings" | ||
| ) | ||
|
|
||
| type BaseConfig struct { | ||
| Path string `yaml:"path" json:"path"` | ||
| } | ||
|
|
||
| func (b BaseConfig) IsValid() error { | ||
| if strings.TrimSpace(b.Path) == "" { | ||
| return fmt.Errorf("path must not be empty or whitespace") | ||
| } | ||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| package imagecustomizerlib | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| "github.com/microsoft/azure-linux-image-tools/toolkit/tools/imagecustomizerapi" | ||
| "github.com/microsoft/azure-linux-image-tools/toolkit/tools/internal/file" | ||
| ) | ||
|
|
||
| type ConfigWithBasePath struct { | ||
| *imagecustomizerapi.Config | ||
elainezhao1 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| BaseConfigPath string | ||
| } | ||
|
|
||
| func buildConfigChain(ctx context.Context, rc *ResolvedConfig) ([]*ConfigWithBasePath, error) { | ||
| visited := make(map[string]bool) | ||
| pathStack := []string{} | ||
|
|
||
| configChain, err := buildConfigChainHelper(ctx, rc.Config, rc.BaseConfigPath, visited, pathStack) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return configChain, nil | ||
| } | ||
|
|
||
| func buildConfigChainHelper(ctx context.Context, cfg *imagecustomizerapi.Config, configFilePath string, visited map[string]bool, | ||
| pathStack []string, | ||
| ) ([]*ConfigWithBasePath, error) { | ||
| var chain []*ConfigWithBasePath | ||
|
|
||
| for _, base := range cfg.BaseConfigs { | ||
| absPath := file.GetAbsPathWithBase(configFilePath, base.Path) | ||
|
|
||
| if visited[absPath] { | ||
| return nil, fmt.Errorf("cycle detected in baseConfigs: %v -> %s", pathStack, absPath) | ||
| } | ||
|
|
||
| visited[absPath] = true | ||
| pathStack = append(pathStack, absPath) | ||
|
|
||
| // Load base file into struct | ||
| var baseCfg imagecustomizerapi.Config | ||
| if err := imagecustomizerapi.UnmarshalYamlFile(absPath, &baseCfg); err != nil { | ||
| return nil, fmt.Errorf("failed to load base config (%s):\n%w", absPath, err) | ||
| } | ||
|
|
||
| // Validate base config content | ||
| if err := baseCfg.IsValid(); err != nil { | ||
| return nil, fmt.Errorf("%w (%s):\n%w", ErrInvalidBaseConfigs, absPath, err) | ||
| } | ||
|
|
||
| // Recurse into base config | ||
| subChain, err := buildConfigChainHelper(ctx, &baseCfg, absPath, visited, pathStack) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| chain = append(chain, subChain...) | ||
| } | ||
|
|
||
| // Add the current config last | ||
| chain = append(chain, &ConfigWithBasePath{ | ||
| Config: cfg, | ||
| BaseConfigPath: configFilePath, | ||
| }) | ||
|
|
||
| return chain, nil | ||
| } | ||
|
|
||
| func resolveOutputArtifacts(configChain []*ConfigWithBasePath) *imagecustomizerapi.Artifacts { | ||
elainezhao1 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| var artifacts *imagecustomizerapi.Artifacts | ||
|
|
||
| for _, configWithBase := range configChain { | ||
| if configWithBase.Config.Output.Artifacts != nil { | ||
| if artifacts == nil { | ||
| artifacts = &imagecustomizerapi.Artifacts{} | ||
| } | ||
|
|
||
| // Artifacts path from current config overrides previous one | ||
| if configWithBase.Config.Output.Artifacts.Path != "" { | ||
| artifacts.Path = file.GetAbsPathWithBase( | ||
| configWithBase.BaseConfigPath, | ||
| configWithBase.Config.Output.Artifacts.Path, | ||
| ) | ||
| } | ||
|
|
||
| // Append items | ||
| artifacts.Items = mergeOutputArtifactTypes( | ||
| artifacts.Items, | ||
| configWithBase.Config.Output.Artifacts.Items, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| return artifacts | ||
| } | ||
|
|
||
| func mergeOutputArtifactTypes(base, current []imagecustomizerapi.OutputArtifactsItemType, | ||
| ) []imagecustomizerapi.OutputArtifactsItemType { | ||
| seen := make(map[imagecustomizerapi.OutputArtifactsItemType]bool) | ||
| var merged []imagecustomizerapi.OutputArtifactsItemType | ||
|
|
||
| // Add base items first | ||
| for _, item := range base { | ||
| if !seen[item] { | ||
| merged = append(merged, item) | ||
| seen[item] = true | ||
| } | ||
| } | ||
|
|
||
| // Add current items | ||
| for _, item := range current { | ||
| if !seen[item] { | ||
| merged = append(merged, item) | ||
| seen[item] = true | ||
| } | ||
| } | ||
|
|
||
| return merged | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| package imagecustomizerlib | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/microsoft/azure-linux-image-tools/toolkit/tools/imagecustomizerapi" | ||
| "github.com/microsoft/azure-linux-image-tools/toolkit/tools/internal/file" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestBaseConfigIsValidNoPath(t *testing.T) { | ||
| base := imagecustomizerapi.BaseConfig{ | ||
| Path: "", | ||
| } | ||
| err := base.IsValid() | ||
elainezhao1 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| assert.Error(t, err) | ||
| assert.ErrorContains(t, err, "path must not be empty or whitespace") | ||
| } | ||
|
|
||
| func TestBaseConfigIsValidWhitespaces(t *testing.T) { | ||
| base := imagecustomizerapi.BaseConfig{ | ||
| Path: " ", | ||
| } | ||
| err := base.IsValid() | ||
| assert.Error(t, err) | ||
| assert.ErrorContains(t, err, "path must not be empty or whitespace") | ||
| } | ||
|
|
||
| func TestBaseConfigsInputAndOutput(t *testing.T) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be good to have at least one test that actually runs through an actual image customization. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to add a full run functional test, any suggestions on how I can obtain a test image from which I can extract 2 artifacts (to test merged artifact items) ? I think current empty.vhdx would not work. if it is not ok to check in a non empty vhdx, can I exclude the artifacts item in the full run? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the You can pass in the base image to the test using the |
||
| testTempDir := filepath.Join(tmpDir, "TestBaseConfigsInputAndOutput") | ||
| defer os.RemoveAll(testTempDir) | ||
|
|
||
| buildDir := filepath.Join(testTempDir, "build") | ||
| currentConfigFile := filepath.Join(testDir, "current-config.yaml") | ||
|
|
||
| options := ImageCustomizerOptions{ | ||
| BuildDir: buildDir, | ||
| } | ||
|
|
||
| var config imagecustomizerapi.Config | ||
| err := imagecustomizerapi.UnmarshalYamlFile(currentConfigFile, &config) | ||
| assert.NoError(t, err) | ||
|
|
||
| rc, err := ValidateConfig(t.Context(), testDir, &config, false, options) | ||
| assert.NoError(t, err) | ||
|
|
||
| // Verify resolved values | ||
| expectedInputPath := file.GetAbsPathWithBase(testDir, "testimages/empty.vhdx") | ||
| expectedOutputPath := file.GetAbsPathWithBase(testDir, "./out/output-image-2.vhdx") | ||
| expectedArtifactsPath := file.GetAbsPathWithBase(testDir, "./artifacts-2") | ||
|
|
||
| assert.Equal(t, expectedInputPath, rc.InputImageFile) | ||
| assert.Equal(t, expectedOutputPath, rc.OutputImageFile) | ||
| assert.Equal(t, expectedArtifactsPath, rc.Config.Output.Artifacts.Path) | ||
| assert.Equal(t, "testname", rc.Config.OS.Hostname) | ||
|
|
||
| // Verify merged artifact items | ||
| expectedItems := []imagecustomizerapi.OutputArtifactsItemType{ | ||
| imagecustomizerapi.OutputArtifactsItemUkis, | ||
| imagecustomizerapi.OutputArtifactsItemShim, | ||
| } | ||
| actual := rc.Config.Output.Artifacts.Items | ||
| assert.Equal(t, len(expectedItems), len(actual)) | ||
|
|
||
| for _, item := range expectedItems { | ||
| assert.Containsf(t, actual, item, "expected output artifact item %q not found in resolved config: %v", | ||
elainezhao1 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| item, actual) | ||
| } | ||
| } | ||
|
|
||
| func TestBaseConfigsMalformed(t *testing.T) { | ||
| testTempDir := filepath.Join(tmpDir, "TestBaseConfigsMalformed") | ||
| defer os.RemoveAll(testTempDir) | ||
|
|
||
| buildDir := filepath.Join(testTempDir, "build") | ||
| currentConfigFile := filepath.Join(testDir, "current-config-malformed.yaml") | ||
|
|
||
| options := ImageCustomizerOptions{ | ||
| BuildDir: buildDir, | ||
| } | ||
|
|
||
| var config imagecustomizerapi.Config | ||
| err := imagecustomizerapi.UnmarshalYamlFile(currentConfigFile, &config) | ||
| assert.NoError(t, err) | ||
|
|
||
| _, err = ValidateConfig(t.Context(), testDir, &config, false, options) | ||
|
|
||
| assert.ErrorContains(t, err, ErrInvalidBaseConfigs.Error()) | ||
elainezhao1 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.