Skip to content

Commit bf9ef0c

Browse files
committed
fix: make sure fill defualts and expand args for all types
Adds tests and fixes implementations where we weren't filling default values and expanding args. This was mostly a problem for anything under the `targets` section, but also a few other fields where arg expansion is nice to have (such as in test commands). Signed-off-by: Brian Goff <[email protected]>
1 parent 48f83f5 commit bf9ef0c

File tree

9 files changed

+1157
-685
lines changed

9 files changed

+1157
-685
lines changed

deps.go

+246
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
package dalec
2+
3+
import (
4+
goerrors "errors"
5+
"slices"
6+
7+
"github.com/pkg/errors"
8+
)
9+
10+
// PackageConstraints is used to specify complex constraints for a package dependency.
11+
type PackageConstraints struct {
12+
// Version is a list of version constraints for the package.
13+
// The format of these strings is dependent on the package manager of the target system.
14+
// Examples:
15+
// [">=1.0.0", "<2.0.0"]
16+
Version []string `yaml:"version,omitempty" json:"version,omitempty"`
17+
// Arch is a list of architecture constraints for the package.
18+
// Use this to specify that a package constraint only applies to certain architectures.
19+
Arch []string `yaml:"arch,omitempty" json:"arch,omitempty"`
20+
}
21+
22+
// PackageDependencies is a list of dependencies for a package.
23+
// This will be included in the package metadata so that the package manager can install the dependencies.
24+
// It also includes build-time dedendencies, which we'll install before running any build steps.
25+
type PackageDependencies struct {
26+
// Build is the list of packagese required to build the package.
27+
Build map[string]PackageConstraints `yaml:"build,omitempty" json:"build,omitempty"`
28+
// Runtime is the list of packages required to install/run the package.
29+
Runtime map[string]PackageConstraints `yaml:"runtime,omitempty" json:"runtime,omitempty"`
30+
// Recommends is the list of packages recommended to install with the generated package.
31+
// Note: Not all package managers support this (e.g. rpm)
32+
Recommends map[string]PackageConstraints `yaml:"recommends,omitempty" json:"recommends,omitempty"`
33+
34+
// Test lists any extra packages required for running tests
35+
// These packages are only installed for tests which have steps that require
36+
// running a command in the built container.
37+
// See [TestSpec] for more information.
38+
Test []string `yaml:"test,omitempty" json:"test,omitempty"`
39+
40+
// ExtraRepos is used to inject extra package repositories that may be used to
41+
// satisfy package dependencies in various stages.
42+
ExtraRepos []PackageRepositoryConfig `yaml:"extra_repos,omitempty" json:"extra_repos,omitempty"`
43+
}
44+
45+
// PackageRepositoryConfig
46+
type PackageRepositoryConfig struct {
47+
// Keys are the list of keys that need to be imported to use the configured
48+
// repositories
49+
Keys map[string]Source `yaml:"keys,omitempty" json:"keys,omitempty"`
50+
51+
// Config list of repo configs to to add to the environment. The format of
52+
// these configs are distro specific (e.g. apt/yum configs).
53+
Config map[string]Source `yaml:"config" json:"config"`
54+
55+
// Data lists all the extra data that needs to be made available for the
56+
// provided repository config to work.
57+
// As an example, if the provided config is referencing a file backed repository
58+
// then data would include the file data, assuming its not already available
59+
// in the environment.
60+
Data []SourceMount `yaml:"data,omitempty" json:"data,omitempty"`
61+
// Envs specifies the list of environments to make the repositories available
62+
// during.
63+
// Acceptable values are:
64+
// - "build" - Repositories are added prior to installing build dependencies
65+
// - "test" - Repositories are added prior to installing test dependencies
66+
// - "install" - Repositories are added prior to installing the output
67+
// package in a container build target.
68+
Envs []string `yaml:"envs" json:"envs" jsonschema:"enum=build,enum=test,enum=install"`
69+
}
70+
71+
func (d *PackageDependencies) processBuildArgs(args map[string]string, allowArg func(string) bool) error {
72+
if d == nil {
73+
return nil
74+
}
75+
76+
var errs []error
77+
for i, repo := range d.ExtraRepos {
78+
if err := repo.processBuildArgs(args, allowArg); err != nil {
79+
errs = append(errs, errors.Wrapf(err, "extra repos index %d", i))
80+
}
81+
d.ExtraRepos[i] = repo
82+
}
83+
return goerrors.Join(errs...)
84+
}
85+
86+
func (r *PackageRepositoryConfig) processBuildArgs(args map[string]string, allowArg func(string) bool) error {
87+
if r == nil {
88+
return nil
89+
}
90+
91+
var errs []error
92+
93+
for k := range r.Config {
94+
src := r.Config[k]
95+
if err := src.processBuildArgs(args, allowArg); err != nil {
96+
errs = append(errs, errors.Wrapf(err, "config %s", k))
97+
continue
98+
}
99+
r.Config[k] = src
100+
}
101+
102+
for k := range r.Keys {
103+
src := r.Keys[k]
104+
if err := src.processBuildArgs(args, allowArg); err != nil {
105+
errs = append(errs, errors.Wrapf(err, "key %s", k))
106+
continue
107+
}
108+
r.Keys[k] = src
109+
}
110+
111+
for i := range r.Data {
112+
d := r.Data[i]
113+
if err := d.processBuildArgs(args, allowArg); err != nil {
114+
errs = append(errs, errors.Wrapf(err, "data index %d", i))
115+
continue
116+
}
117+
r.Data[i] = d
118+
}
119+
120+
return goerrors.Join(errs...)
121+
}
122+
123+
func (d *PackageDependencies) fillDefaults() {
124+
if d == nil {
125+
return
126+
}
127+
128+
for i, r := range d.ExtraRepos {
129+
r.fillDefaults()
130+
d.ExtraRepos[i] = r
131+
}
132+
}
133+
134+
func (r *PackageRepositoryConfig) fillDefaults() {
135+
if len(r.Envs) == 0 {
136+
// default to all stages for the extra repo if unspecified
137+
r.Envs = []string{"build", "install", "test"}
138+
}
139+
140+
for i, src := range r.Config {
141+
fillDefaults(&src)
142+
r.Config[i] = src
143+
}
144+
145+
for i, src := range r.Keys {
146+
fillDefaults(&src)
147+
148+
// Default to 0644 permissions for gpg keys. This is because apt will will only import
149+
// keys with a particular permission set.
150+
if src.HTTP != nil {
151+
src.HTTP.Permissions = 0644
152+
}
153+
r.Keys[i] = src
154+
}
155+
156+
for i, mount := range r.Data {
157+
mount.fillDefaults()
158+
r.Data[i] = mount
159+
}
160+
}
161+
162+
func (d *PackageDependencies) validate() error {
163+
if d == nil {
164+
return nil
165+
}
166+
167+
var errs []error
168+
for i, r := range d.ExtraRepos {
169+
if err := r.validate(); err != nil {
170+
errs = append(errs, errors.Wrapf(err, "extra repo %d", i))
171+
}
172+
}
173+
174+
return goerrors.Join(errs...)
175+
}
176+
177+
func (r *PackageRepositoryConfig) validate() error {
178+
var errs []error
179+
for name, src := range r.Keys {
180+
if err := src.validate(); err != nil {
181+
errs = append(errs, errors.Wrapf(err, "key %s", name))
182+
}
183+
}
184+
for name, src := range r.Config {
185+
if err := src.validate(); err != nil {
186+
errs = append(errs, errors.Wrapf(err, "config %s", name))
187+
}
188+
}
189+
for _, mnt := range r.Data {
190+
if err := mnt.validate("/"); err != nil {
191+
errs = append(errs, errors.Wrapf(err, "data mount path %s", mnt.Dest))
192+
}
193+
}
194+
195+
return goerrors.Join(errs...)
196+
}
197+
198+
func (p *PackageDependencies) GetExtraRepos(env string) []PackageRepositoryConfig {
199+
return GetExtraRepos(p.ExtraRepos, env)
200+
}
201+
202+
func GetExtraRepos(repos []PackageRepositoryConfig, env string) []PackageRepositoryConfig {
203+
var out []PackageRepositoryConfig
204+
for _, repo := range repos {
205+
if slices.Contains(repo.Envs, env) {
206+
out = append(repos, repo)
207+
}
208+
}
209+
return out
210+
}
211+
212+
func (s *Spec) GetBuildRepos(targetKey string) []PackageRepositoryConfig {
213+
deps := s.GetPackageDeps(targetKey)
214+
if deps == nil {
215+
deps = s.Dependencies
216+
if deps == nil {
217+
return nil
218+
}
219+
}
220+
221+
return deps.GetExtraRepos("build")
222+
}
223+
224+
func (s *Spec) GetInstallRepos(targetKey string) []PackageRepositoryConfig {
225+
deps := s.GetPackageDeps(targetKey)
226+
if deps == nil {
227+
deps = s.Dependencies
228+
if deps == nil {
229+
return nil
230+
}
231+
}
232+
233+
return deps.GetExtraRepos("install")
234+
}
235+
236+
func (s *Spec) GetTestRepos(targetKey string) []PackageRepositoryConfig {
237+
deps := s.GetPackageDeps(targetKey)
238+
if deps == nil {
239+
deps = s.Dependencies
240+
if deps == nil {
241+
return nil
242+
}
243+
}
244+
245+
return deps.GetExtraRepos("test")
246+
}

frontend/mux.go

+3-6
Original file line numberDiff line numberDiff line change
@@ -205,12 +205,9 @@ func (m *BuildMux) loadSpec(ctx context.Context, client gwclient.Client) (*dalec
205205

206206
// Note: this is not suitable for passing to builds since it does not have platform information
207207
spec, err := LoadSpec(ctx, dc, nil, func(cfg *LoadConfig) {
208-
cfg.SubstituteOpts = append(cfg.SubstituteOpts, func(cfg *dalec.SubstituteConfig) {
209-
// Allow any args here since we aren't trying to validate the spec at this point.
210-
cfg.AllowArg = func(string) bool {
211-
return true
212-
}
213-
})
208+
// We want to allow any arg to be passed to the spec since we don't know what
209+
// args are valid at this point, nor do we care here.
210+
cfg.SubstituteOpts = append(cfg.SubstituteOpts, dalec.WithAllowAnyArg)
214211
})
215212
if err != nil {
216213
return nil, err

helpers.go

-50
Original file line numberDiff line numberDiff line change
@@ -324,42 +324,6 @@ func (s *Spec) GetBuildDeps(targetKey string) map[string]PackageConstraints {
324324
return deps.Build
325325
}
326326

327-
func (s *Spec) GetBuildRepos(targetKey string) []PackageRepositoryConfig {
328-
deps := s.GetPackageDeps(targetKey)
329-
if deps == nil {
330-
deps = s.Dependencies
331-
if deps == nil {
332-
return nil
333-
}
334-
}
335-
336-
return deps.GetExtraRepos("build")
337-
}
338-
339-
func (s *Spec) GetInstallRepos(targetKey string) []PackageRepositoryConfig {
340-
deps := s.GetPackageDeps(targetKey)
341-
if deps == nil {
342-
deps = s.Dependencies
343-
if deps == nil {
344-
return nil
345-
}
346-
}
347-
348-
return deps.GetExtraRepos("install")
349-
}
350-
351-
func (s *Spec) GetTestRepos(targetKey string) []PackageRepositoryConfig {
352-
deps := s.GetPackageDeps(targetKey)
353-
if deps == nil {
354-
deps = s.Dependencies
355-
if deps == nil {
356-
return nil
357-
}
358-
}
359-
360-
return deps.GetExtraRepos("test")
361-
}
362-
363327
func (s *Spec) GetTestDeps(targetKey string) []string {
364328
var deps *PackageDependencies
365329
if t, ok := s.Targets[targetKey]; ok {
@@ -623,17 +587,3 @@ func BaseImageConfig(platform *ocispecs.Platform) *DockerImageSpec {
623587

624588
return img
625589
}
626-
627-
func (p *PackageDependencies) GetExtraRepos(env string) []PackageRepositoryConfig {
628-
return GetExtraRepos(p.ExtraRepos, env)
629-
}
630-
631-
func GetExtraRepos(repos []PackageRepositoryConfig, env string) []PackageRepositoryConfig {
632-
var out []PackageRepositoryConfig
633-
for _, repo := range repos {
634-
if slices.Contains(repo.Envs, env) {
635-
out = append(repos, repo)
636-
}
637-
}
638-
return out
639-
}

0 commit comments

Comments
 (0)