Skip to content
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.

Sign up for GitHub

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

Disable subobjects by default on new packages and data streams #1965

Merged
merged 2 commits into from
Jul 11, 2024
Merged

Disable subobjects by default on new packages and data streams #1965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67d00e3
Disable subobjects by default on new packages
jsoriano Jul 10, 2024
c5a4533
Add tests
jsoriano Jul 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 26 additions & 10 deletions cmd/create_data_stream.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type newDataStreamAnswers struct {
Name string
Title string
Type string
Subobjects bool
SyntheticAndTimeSeries bool
Synthetic bool
}
Expand Down Expand Up @@ -75,6 +76,14 @@ func createDataStreamCommandAction(cmd *cobra.Command, args []string) error {
},
Validate: survey.Required,
},
{
Name: "subobjects",
Prompt: &survey.Confirm{
Message: "Enable creation of subobjects for fields with dots in their names?",
Default: false,
},
Validate: survey.Required,
},
}
var answers newDataStreamAnswers
err = survey.Ask(qs, &answers)
Expand Down Expand Up @@ -133,19 +142,26 @@ func createDataStreamDescriptorFromAnswers(answers newDataStreamAnswers, package
Type: answers.Type,
}

if !answers.SyntheticAndTimeSeries && !answers.Synthetic {
return archetype.DataStreamDescriptor{
Manifest: manifest,
PackageRoot: packageRoot,
if !answers.Subobjects {
manifest.Elasticsearch = &packages.Elasticsearch{
IndexTemplate: &packages.ManifestIndexTemplate{
Mappings: &packages.ManifestMappings{
Subobjects: false,
},
},
}
}
elasticsearch := packages.Elasticsearch{
SourceMode: "synthetic",
}
if answers.SyntheticAndTimeSeries {
elasticsearch.IndexMode = "time_series"

if answers.Synthetic || answers.SyntheticAndTimeSeries {
if manifest.Elasticsearch == nil {
manifest.Elasticsearch = &packages.Elasticsearch{}
}
manifest.Elasticsearch.SourceMode = "synthetic"
if answers.SyntheticAndTimeSeries {
manifest.Elasticsearch.IndexMode = "time_series"
}
}
manifest.Elasticsearch = &elasticsearch

return archetype.DataStreamDescriptor{
Manifest: manifest,
PackageRoot: packageRoot,
Expand Down
27 changes: 24 additions & 3 deletions cmd/create_package.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type newPackageAnswers struct {
GithubOwner string `survey:"github_owner"`
OwnerType string `survey:"owner_type"`
DataStreamType string `survey:"datastream_type"`
Subobjects bool
}

func createPackageCommandAction(cmd *cobra.Command, args []string) error {
Expand Down Expand Up @@ -184,6 +185,14 @@ func createPackageCommandAction(cmd *cobra.Command, args []string) error {
},
Validate: survey.Required,
},
{
Name: "subobjects",
Prompt: &survey.Confirm{
Message: "Enable creation of subobjects for fields with dots in their names?",
Default: false,
},
Validate: survey.Required,
},
}

qs = append(qs, inputQs...)
Expand Down Expand Up @@ -216,10 +225,21 @@ func createPackageDescriptorFromAnswers(answers newPackageAnswers) archetype.Pac
sourceLicense = answers.SourceLicense
}

var elasticsearch *packages.Elasticsearch
inputDataStreamType := ""
if answers.Type == "input" {
inputDataStreamType = answers.DataStreamType
if !answers.Subobjects {
elasticsearch = &packages.Elasticsearch{
IndexTemplate: &packages.ManifestIndexTemplate{
Mappings: &packages.ManifestMappings{
Subobjects: false,
},
},
}
}
}

return archetype.PackageDescriptor{
Manifest: packages.PackageManifest{
Name: answers.Name,
Expand All @@ -241,9 +261,10 @@ func createPackageDescriptorFromAnswers(answers newPackageAnswers) archetype.Pac
Github: answers.GithubOwner,
Type: answers.OwnerType,
},
License: answers.ElasticSubscription,
Description: answers.Description,
Categories: answers.Categories,
License: answers.ElasticSubscription,
Description: answers.Description,
Categories: answers.Categories,
Elasticsearch: elasticsearch,
},
InputDataStreamType: inputDataStreamType,
}
Expand Down
13 changes: 10 additions & 3 deletions internal/packages/archetype/_static/dataStream-manifest.yml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,20 @@ streams:{{if eq .Manifest.Type "logs" }}
type: text
title: Period
default: 10s
{{- end }}
{{ if .Manifest.Elasticsearch }}
elasticsearch:
{{ if .Manifest.Elasticsearch.SourceMode }}
source_mode: {{ .Manifest.Elasticsearch.SourceMode }}
{{- end}}
{{ if .Manifest.Elasticsearch.IndexMode }}
index_mode: {{ .Manifest.Elasticsearch.IndexMode }}
{{- end}}
{{- end}}
{{- end}}
{{- end }}
{{ if .Manifest.Elasticsearch.IndexTemplate }}
index_template:
{{ if .Manifest.Elasticsearch.IndexTemplate.Mappings }}
mappings:
subobjects: {{ .Manifest.Elasticsearch.IndexTemplate.Mappings.Subobjects }}
{{- end }}
{{- end }}
{{- end }}
16 changes: 16 additions & 0 deletions internal/packages/archetype/_static/package-manifest.yml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,22 @@ policy_templates:
- localhost
{{ end }}
{{ end -}}
{{ if .Manifest.Elasticsearch }}
elasticsearch:
{{ if .Manifest.Elasticsearch.SourceMode }}
source_mode: {{ .Manifest.Elasticsearch.SourceMode }}
{{- end}}
{{ if .Manifest.Elasticsearch.IndexMode }}
index_mode: {{ .Manifest.Elasticsearch.IndexMode }}
{{- end }}
Comment on lines +77 to +82
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, there are no questions for these values when creating the package. So, right now they would not be written here in this manifest.

Should they be added also as part of the questions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, I could add them in a follow up if necessary. Or would you prefer the questions to be added here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, they can be added in a follow up, it's not needed to do it now.

{{ if .Manifest.Elasticsearch.IndexTemplate }}
index_template:
{{ if .Manifest.Elasticsearch.IndexTemplate.Mappings }}
mappings:
subobjects: {{ .Manifest.Elasticsearch.IndexTemplate.Mappings.Subobjects }}
{{- end }}
{{- end }}
{{- end }}
owner:
github: {{.Manifest.Owner.Github}}
type: {{.Manifest.Owner.Type}}
24 changes: 15 additions & 9 deletions internal/packages/archetype/data_stream_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,38 +20,45 @@ func TestDataStream(t *testing.T) {
dd := createDataStreamDescriptorForTest()
dd.Manifest.Type = "logs"

err := createAndCheckDataStream(t, pd, dd)
require.NoError(t, err)
createAndCheckDataStream(t, pd, dd, true)
})
t.Run("valid-metrics", func(t *testing.T) {
pd := createPackageDescriptorForTest("integration", "^7.13.0")
dd := createDataStreamDescriptorForTest()
dd.Manifest.Type = "metrics"

err := createAndCheckDataStream(t, pd, dd)
require.NoError(t, err)
createAndCheckDataStream(t, pd, dd, true)
})
t.Run("missing-type", func(t *testing.T) {
pd := createPackageDescriptorForTest("integration", "^7.13.0")
dd := createDataStreamDescriptorForTest()
dd.Manifest.Type = ""

err := createAndCheckDataStream(t, pd, dd)
require.Error(t, err)
createAndCheckDataStream(t, pd, dd, false)
})
}

func createDataStreamDescriptorForTest() DataStreamDescriptor {
elasticsearch := &packages.Elasticsearch{
IndexTemplate: &packages.ManifestIndexTemplate{
Mappings: &packages.ManifestMappings{
Subobjects: false,
},
},
}

return DataStreamDescriptor{
Manifest: packages.DataStreamManifest{
Name: "go_unit_test_data_stream",
Title: "Go Unit Test Data Stream",
Type: "logs",

Elasticsearch: elasticsearch,
},
}
}

func createAndCheckDataStream(t require.TestingT, pd PackageDescriptor, dd DataStreamDescriptor) error {
func createAndCheckDataStream(t *testing.T, pd PackageDescriptor, dd DataStreamDescriptor, valid bool) {
wd, err := os.Getwd()
require.NoError(t, err)

Expand All @@ -73,6 +80,5 @@ func createAndCheckDataStream(t require.TestingT, pd PackageDescriptor, dd DataS
err = CreateDataStream(dd)
require.NoError(t, err)

err = checkPackage(pd.Manifest.Name)
return err
checkPackage(t, pd.Manifest.Name, valid)
}
67 changes: 48 additions & 19 deletions internal/packages/archetype/package_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
package archetype

import (
"fmt"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/elastic/elastic-package/internal/packages"
Expand All @@ -18,38 +18,39 @@ import (
func TestPackage(t *testing.T) {
t.Run("valid", func(t *testing.T) {
pd := createPackageDescriptorForTest("integration", "^7.13.0")

err := createAndCheckPackage(t, pd)
require.NoError(t, err)
createAndCheckPackage(t, pd, true)
})
t.Run("missing-version", func(t *testing.T) {
pd := createPackageDescriptorForTest("integration", "^7.13.0")
pd.Manifest.Version = ""

err := createAndCheckPackage(t, pd)
require.Error(t, err)
createAndCheckPackage(t, pd, false)
})
t.Run("input-package", func(t *testing.T) {
pd := createPackageDescriptorForTest("input", "^8.9.0")

err := createAndCheckPackage(t, pd)
require.NoError(t, err)
createAndCheckPackage(t, pd, true)
})
}

func createAndCheckPackage(t *testing.T, pd PackageDescriptor) error {
func createAndCheckPackage(t *testing.T, pd PackageDescriptor, valid bool) {
tempDir := t.TempDir()
err := createPackageInDir(pd, tempDir)
require.NoError(t, err)

err = checkPackage(filepath.Join(tempDir, pd.Manifest.Name))
return err
checkPackage(t, filepath.Join(tempDir, pd.Manifest.Name), valid)
}

func createPackageDescriptorForTest(packageType, kibanaVersion string) PackageDescriptor {
var elasticsearch *packages.Elasticsearch
inputDataStreamType := ""
if packageType == "input" {
inputDataStreamType = "logs"
elasticsearch = &packages.Elasticsearch{
IndexTemplate: &packages.ManifestIndexTemplate{
Mappings: &packages.ManifestMappings{
Subobjects: false,
},
},
}
}
specVersion, err := GetLatestStableSpecVersion()
if err != nil {
Expand All @@ -74,17 +75,45 @@ func createPackageDescriptorForTest(packageType, kibanaVersion string) PackageDe
Github: "mtojek",
Type: "elastic",
},
Description: "This package has been generated by a Go unit test.",
Categories: []string{"aws", "custom"},
Description: "This package has been generated by a Go unit test.",
Categories: []string{"aws", "custom"},
Elasticsearch: elasticsearch,
},
InputDataStreamType: inputDataStreamType,
}
}

func checkPackage(packageRoot string) error {
func checkPackage(t *testing.T, packageRoot string, valid bool) {
err := validation.ValidateFromPath(packageRoot)
if err != nil {
return fmt.Errorf("linting package failed: %w", err)
if !valid {
assert.Error(t, err)
return
}
require.NoError(t, err)

manifest, err := packages.ReadPackageManifestFromPackageRoot(packageRoot)
require.NoError(t, err)

// Running in subtests because manifest subobjects can be pointers that can panic when dereferenced by assertions.
if manifest.Type == "input" {
t.Run("input", func(t *testing.T) {
t.Run("subobjects expected to false", func(t *testing.T) {
assert.False(t, manifest.Elasticsearch.IndexTemplate.Mappings.Subobjects)
})
})
}

if manifest.Type == "integration" {
t.Run("integration", func(t *testing.T) {
ds, err := filepath.Glob(filepath.Join(packageRoot, "data_stream", "*"))
require.NoError(t, err)
for _, d := range ds {
manifest, err := packages.ReadDataStreamManifest(filepath.Join(d, "manifest.yml"))
require.NoError(t, err)
t.Run("subobjects expected to false", func(t *testing.T) {
assert.False(t, manifest.Elasticsearch.IndexTemplate.Mappings.Subobjects)
})
}
})
}
return nil
}
24 changes: 17 additions & 7 deletions internal/packages/packages.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,16 +137,26 @@ type PackageManifest struct {
License string `config:"license" json:"license" yaml:"license"`
Categories []string `config:"categories" json:"categories" yaml:"categories"`
Agent Agent `config:"agent" json:"agent" yaml:"agent"`
Elasticsearch *Elasticsearch `config:"elasticsearch" json:"elasticsearch" yaml:"elasticsearch"`
}

type ManifestIndexTemplate struct {
IngestPipeline *ManifestIngestPipeline `config:"ingest_pipeline" json:"ingest_pipeline" yaml:"ingest_pipeline"`
Mappings *ManifestMappings `config:"mappings" json:"mappings" yaml:"mappings"`
}

type ManifestIngestPipeline struct {
Name string `config:"name" json:"name" yaml:"name"`
}

type ManifestMappings struct {
Subobjects bool `config:"subobjects" json:"subobjects" yaml:"subobjects"`
}

type Elasticsearch struct {
IndexTemplate *struct {
IngestPipeline *struct {
Name string `config:"name" json:"name" yaml:"name"`
} `config:"ingest_pipeline" json:"ingest_pipeline" yaml:"ingest_pipeline"`
} `config:"index_template" json:"index_template" yaml:"index_template"`
SourceMode string `config:"source_mode" json:"source_mode" yaml:"source_mode"`
IndexMode string `config:"index_mode" json:"index_mode" yaml:"index_mode"`
IndexTemplate *ManifestIndexTemplate `config:"index_template" json:"index_template" yaml:"index_template"`
SourceMode string `config:"source_mode" json:"source_mode" yaml:"source_mode"`
IndexMode string `config:"index_mode" json:"index_mode" yaml:"index_mode"`
}

// DataStreamManifest represents the structure of a data stream's manifest
Expand Down