-
Notifications
You must be signed in to change notification settings - Fork 66
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
Switch for disabling validation #667
Changes from all commits
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 | ||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -151,22 +151,11 @@ func NewDataStream(basePath string, p *Package) (*DataStream, error) { | |||||||||||||||||||||||||||||||||||||||||||||||
if !IsValidRelease(d.Release) { | ||||||||||||||||||||||||||||||||||||||||||||||||
return nil, fmt.Errorf("invalid release: %s", d.Release) | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
return d, nil | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
func (d *DataStream) Validate() error { | ||||||||||||||||||||||||||||||||||||||||||||||||
pipelineDir := filepath.Join(d.BasePath, "elasticsearch", DirIngestPipeline) | ||||||||||||||||||||||||||||||||||||||||||||||||
paths, err := filepath.Glob(filepath.Join(pipelineDir, "*")) | ||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||
return err | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
if strings.Contains(d.Dataset, "-") { | ||||||||||||||||||||||||||||||||||||||||||||||||
return fmt.Errorf("data stream name is not allowed to contain `-`: %s", d.Dataset) | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
if !d.validType() { | ||||||||||||||||||||||||||||||||||||||||||||||||
return fmt.Errorf("type is not valid: %s", d.Type) | ||||||||||||||||||||||||||||||||||||||||||||||||
return nil, err | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
if d.Elasticsearch != nil && d.Elasticsearch.IngestPipelineName == "" { | ||||||||||||||||||||||||||||||||||||||||||||||||
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. This is kind of a mix between validation and building the data model. I wonder if we should clean it up later or keep it as is. It would probably mean doing parts of it twice. 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. I'm not sure if I understand your concerns. Any code that modifies a package or a datastream is placed in constructors (e.g. package-registry/util/data_stream.go Lines 161 to 183 in b20850d
DataStream.IngestPipeline is marked as deprecated.
I suppose we can strip this part of at all, unless there is Kibana logic using it. 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 I initially assume could be an issue does not seem to be one after quickly checking the code. I was worried that for example the pipeline checks do not happen if only 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. Yeah, I know the design is not really clear as we have to sneak the condition into the 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. I think I know the person who built this initially and is to blame for the design 😆 |
||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -189,9 +178,25 @@ func (d *DataStream) Validate() error { | |||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
if d.IngestPipeline == "" && len(paths) > 0 { | ||||||||||||||||||||||||||||||||||||||||||||||||
return fmt.Errorf("unused pipelines in the package (dataset: %s): %s", d.Dataset, strings.Join(paths, ",")) | ||||||||||||||||||||||||||||||||||||||||||||||||
return nil, fmt.Errorf("unused pipelines in the package (dataset: %s): %s", d.Dataset, strings.Join(paths, ",")) | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
return d, nil | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
func (d *DataStream) Validate() error { | ||||||||||||||||||||||||||||||||||||||||||||||||
if PackageValidationDisabled { | ||||||||||||||||||||||||||||||||||||||||||||||||
return nil | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
pipelineDir := filepath.Join(d.BasePath, "elasticsearch", DirIngestPipeline) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
if strings.Contains(d.Dataset, "-") { | ||||||||||||||||||||||||||||||||||||||||||||||||
return fmt.Errorf("data stream name is not allowed to contain `-`: %s", d.Dataset) | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
if !d.validType() { | ||||||||||||||||||||||||||||||||||||||||||||||||
return fmt.Errorf("type is not valid: %s", d.Type) | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
// In case an ingest pipeline is set, check if it is around | ||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -204,7 +209,7 @@ func (d *DataStream) Validate() error { | |||||||||||||||||||||||||||||||||||||||||||||||
return errors.Wrapf(errJSON, "stat ingest pipeline JSON file failed (path: %s)", jsonPipelinePath) | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
if !os.IsNotExist(errJSON) { | ||||||||||||||||||||||||||||||||||||||||||||||||
err = validateIngestPipelineFile(jsonPipelinePath) | ||||||||||||||||||||||||||||||||||||||||||||||||
err := validateIngestPipelineFile(jsonPipelinePath) | ||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||
return errors.Wrapf(err, "validating ingest pipeline JSON file failed (path: %s)", jsonPipelinePath) | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -217,7 +222,7 @@ func (d *DataStream) Validate() error { | |||||||||||||||||||||||||||||||||||||||||||||||
return errors.Wrapf(errYAML, "stat ingest pipeline YAML file failed (path: %s)", jsonPipelinePath) | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
if !os.IsNotExist(errYAML) { | ||||||||||||||||||||||||||||||||||||||||||||||||
err = validateIngestPipelineFile(yamlPipelinePath) | ||||||||||||||||||||||||||||||||||||||||||||||||
err := validateIngestPipelineFile(yamlPipelinePath) | ||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||
return errors.Wrapf(err, "validating ingest pipeline YAML file failed (path: %s)", jsonPipelinePath) | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -229,7 +234,7 @@ func (d *DataStream) Validate() error { | |||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||
err = d.validateRequiredFields() | ||||||||||||||||||||||||||||||||||||||||||||||||
err := d.validateRequiredFields() | ||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||
return errors.Wrap(err, "validating required fields failed") | ||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
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.
Quite a long name for a flag but no better suggestion.
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.
It's one of these feature flags that you should be discouraged to use :)