-
Notifications
You must be signed in to change notification settings - Fork 91
Validation rules: build/source mode enforcement #1178
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 47 commits
c2b6ed7
1166296
892c661
a5c8cfc
30cfd76
d482f31
2769edc
324c4a0
d999602
fe83a0a
491658c
15750f5
fcaf8c2
f4e266e
e9d0013
cbd3b7c
0f275c0
d1efd03
4233331
da2a325
8539209
b0049ff
d4508b1
8d441f1
15cc91e
cc5e273
2a3fef8
d8a2a6c
ebac0c7
d895e49
6fd467f
2abdb49
ab9ad08
73326a2
9345254
cc99311
0c34f3f
7b72caf
561b526
8ccac7b
148aebf
8c60420
a52f51a
5b78784
44f5383
b66a1f3
4206c50
dedc2ff
c29ab54
2fdf404
1219cd3
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 |
|---|---|---|
|
|
@@ -352,9 +352,11 @@ func listDataStreams(fsys fspath.FS) ([]string, error) { | |
| return nil, fmt.Errorf("can't list data streams directory: %w", err) | ||
| } | ||
|
|
||
| list := make([]string, len(dataStreams)) | ||
| for i, dataStream := range dataStreams { | ||
| list[i] = dataStream.Name() | ||
| var list []string | ||
| for _, dataStream := range dataStreams { | ||
| if dataStream.IsDir() { | ||
| list = append(list, dataStream.Name()) | ||
| } | ||
|
Contributor
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. What is the motivation of this change? Can we have anything under
Contributor
Author
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. by the spec definition there are only directories on the datastream dir, but, fs.ReadDir is gathering directories and files, so i thought it was interesting to add this guard at this layer to ensure it. based on the docs https://pkg.go.dev/io/fs#DirEntry the direntry interface could be one or the other... |
||
| } | ||
| return list, nil | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,11 +20,11 @@ import ( | |
|
|
||
| const ( | ||
| defaultStreamTemplatePath = "stream.yml.hbs" | ||
| packageTypeIntegration = "integration" | ||
| ) | ||
|
|
||
| type policyTemplateInput struct { | ||
| Type string `yaml:"type"` | ||
| Package string `yaml:"package"` | ||
| TemplatePath string `yaml:"template_path"` | ||
| TemplatePaths []string `yaml:"template_paths"` | ||
| } | ||
|
|
@@ -41,6 +41,7 @@ type integrationPackageManifest struct { // package manifest | |
|
|
||
| type stream struct { | ||
| Input string `yaml:"input"` | ||
| Package string `yaml:"package"` | ||
| TemplatePath string `yaml:"template_path"` | ||
| TemplatePaths []string `yaml:"template_paths"` | ||
| } | ||
|
|
@@ -78,7 +79,7 @@ func ValidateIntegrationPolicyTemplates(fsys fspath.FS) specerrors.ValidationErr | |
| specerrors.NewStructuredErrorf("file \"%s\" is invalid: %w", fsys.Path(manifestPath), errFailedToParseManifest)} | ||
| } | ||
|
|
||
| if manifest.Type != packageTypeIntegration { | ||
| if manifest.Type != integrationPackageType { | ||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -110,6 +111,9 @@ func ValidateIntegrationPolicyTemplates(fsys fspath.FS) specerrors.ValidationErr | |
| // under agent/input when template_paths or template_path is set (Fleet: template_paths first). | ||
| func validateIntegrationPolicyTemplateInputs(fsys fspath.FS, policyTemplate integrationPolicyTemplate) error { | ||
| for _, input := range policyTemplate.Inputs { | ||
| // Only validate template files that are explicitly declared; if none are set there | ||
| // is nothing to check (composable inputs without overlay templates source them from | ||
| // the dependency package, which is absent from the source tree). | ||
|
Contributor
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. Thanks for making these comments more concise 👍 |
||
| if len(input.TemplatePaths) > 0 { | ||
| for _, tp := range input.TemplatePaths { | ||
| if err := validateAgentInputTemplatePath(fsys, tp); err != nil { | ||
|
|
@@ -141,6 +145,11 @@ func validateAllDataStreamStreamTemplates(fsys fspath.FS, dsMap map[string]dataS | |
| dsManifestPath := path.Join(dsDir, "manifest.yml") | ||
| manifest := dsMap[dsDir] | ||
| for _, s := range manifest.Streams { | ||
| // Don't validate template paths if they use a package as input | ||
| // and they don't define any template. | ||
| if s.Package != "" && s.TemplatePath == "" && len(s.TemplatePaths) == 0 { | ||
| continue | ||
| } | ||
| if err := validateSingleDataStreamStreamTemplates(fsys, dsDir, s); err != nil { | ||
| errs = append(errs, specerrors.NewStructuredErrorf( | ||
| "file \"%s\" is invalid: data stream \"%s\" stream input %q: %w", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to see this working 👍 Thanks.
Could we make this more flexible and allow to set the mode the item should appear on? In case we need items that should only appear in built packages.
So instead of setting
sourceOnly: true, we setvalidationMode: source.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, makes sense. i was thinking on this while creating the issue for the followup, to declare the "specific mode" instead of booleans. i will update the pr with these change