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

[Ingest Manager] Split index restrictions into type,dataset, namespace parts #21406

Merged
merged 2 commits into from
Oct 2, 2020
Merged

[Ingest Manager] Split index restrictions into type,dataset, namespace parts #21406

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4d6da1b
restrictions on index split
michalpristas Sep 30, 2020
c8423b3
extract common restrictions
michalpristas Sep 30, 2020
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
103 changes: 62 additions & 41 deletions x-pack/elastic-agent/pkg/agent/application/filters/stream_checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package filters

import (
"fmt"
"strings"

"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/errors"
Expand Down Expand Up @@ -47,11 +46,7 @@ func StreamChecker(log *logger.Logger, ast *transpiler.AST) error {
if nsNode, found := inputNode.Find("data_stream.namespace"); found {
nsKey, ok := nsNode.(*transpiler.Key)
if ok {
newNamespace := nsKey.Value().(transpiler.Node).String()
if !isValid(newNamespace) {
return ErrInvalidNamespace
}
namespace = newNamespace
namespace = nsKey.Value().(transpiler.Node).String()
}
} else {
dsNode, found := inputNode.Find("data_stream")
Expand All @@ -63,17 +58,17 @@ func StreamChecker(log *logger.Logger, ast *transpiler.AST) error {
if found {
nsKey, ok := nsNode.(*transpiler.Key)
if ok {
newNamespace := nsKey.Value().(transpiler.Node).String()
if !isValid(newNamespace) {
return ErrInvalidNamespace
}
namespace = newNamespace
namespace = nsKey.Value().(transpiler.Node).String()
}
}
}
}
}

if !matchesNamespaceContraints(namespace) {
return ErrInvalidNamespace
}

// get the type, longest type for now is metrics
datasetType := "metrics"
if nsNode, found := inputNode.Find("data_stream.type"); found {
Expand All @@ -100,6 +95,10 @@ func StreamChecker(log *logger.Logger, ast *transpiler.AST) error {
}
}

if !matchesTypeConstraints(datasetType) {
return ErrInvalidIndex
}

streamsNode, ok := inputNode.Find("streams")
if ok {
streamsList, ok := streamsNode.Value().(*transpiler.List)
Expand All @@ -119,11 +118,8 @@ func StreamChecker(log *logger.Logger, ast *transpiler.AST) error {
if dsNameNode, found := streamMap.Find("data_stream.dataset"); found {
dsKey, ok := dsNameNode.(*transpiler.Key)
if ok {
newDataset := dsKey.Value().(transpiler.Node).String()
if !isValid(newDataset) {
return ErrInvalidDataset
}
datasetName = newDataset
datasetName = dsKey.Value().(transpiler.Node).String()
break
}
} else {
datasetNode, found := streamMap.Find("data_stream")
Expand All @@ -137,63 +133,88 @@ func StreamChecker(log *logger.Logger, ast *transpiler.AST) error {
if found {
dsKey, ok := dsNameNode.(*transpiler.Key)
if ok {
newDataset := dsKey.Value().(transpiler.Node).String()
if !isValid(newDataset) {
return ErrInvalidDataset
}
datasetName = newDataset
datasetName = dsKey.Value().(transpiler.Node).String()
break
}
}
}
}
}
}
}

if indexName := fmt.Sprintf("%s-%s-%s", datasetType, datasetName, namespace); !matchesIndexContraints(indexName) {
return ErrInvalidIndex
if !matchesDatasetConstraints(datasetName) {
return ErrInvalidDataset
}
}

return nil
}

// The only two requirement are that it has only characters allowed in an Elasticsearch index name
// and does NOT contain a `-`.
func isValid(namespace string) bool {
return matchesIndexContraints(namespace) && !strings.Contains(namespace, "-")
}

// The only two requirement are that it has only characters allowed in an Elasticsearch index name
// Index names must meet the following criteria:
// Not longer than 100 bytes
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Indentation seems to be off

// Lowercase only
// Cannot include \, /, *, ?, ", <, >, |, ` ` (space character), ,, #
func matchesNamespaceContraints(namespace string) bool {
// length restriction is in bytes, not characters
if len(namespace) <= 0 || len(namespace) > 100 {
return false
}

// Lowercase only
if strings.ToLower(namespace) != namespace {
return false
}

// Cannot include \, /, *, ?, ", <, >, |, ` ` (space character), ,, #
if strings.ContainsAny(namespace, "\\/*?\"<>| ,#:") {
return false
}

return true
}

// matchesTypeConstraints fails for following rules. As type is first element of resulting index prefix restrictions need to be applied.
// Not longer than 20 bytes
// Lowercase only
// Cannot start with -, _, +
// Cannot be . or ..
func matchesIndexContraints(namespace string) bool {
// Cannot be . or ..
if namespace == "." || namespace == ".." {
// Cannot include \, /, *, ?, ", <, >, |, ` ` (space character), ,, #
func matchesTypeConstraints(dsType string) bool {
// length restriction is in bytes, not characters
if len(dsType) <= 0 || len(dsType) > 20 {
return false
}

if len(namespace) <= 0 || len(namespace) > 255 {
if strings.ToLower(dsType) != dsType {
return false
}

// Lowercase only
if strings.ToLower(namespace) != namespace {
if strings.HasPrefix(dsType, "-") || strings.HasPrefix(dsType, "_") || strings.HasPrefix(dsType, "+") {
return false
}

// Cannot include \, /, *, ?, ", <, >, |, ` ` (space character), ,, #
if strings.ContainsAny(namespace, "\\/*?\"<>| ,#") {
if strings.ContainsAny(dsType, "\\/*?\"<>| ,#:") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we reuse the logic from dataset / namespace here to make sure it is always in sync?

return false
}
return true
}

// matchesDatasetConstraints fails for following rules
// Not longer than 100 bytes
// Lowercase only
// Cannot include \, /, *, ?, ", <, >, |, ` ` (space character), ,, #
func matchesDatasetConstraints(dataset string) bool {
// length restriction is in bytes, not characters
if len(dataset) <= 0 || len(dataset) > 100 {
return false
}

// Cannot start with -, _, +
if strings.HasPrefix(namespace, "-") || strings.HasPrefix(namespace, "_") || strings.HasPrefix(namespace, "+") {
if strings.ToLower(dataset) != dataset {
return false
}

if strings.ContainsAny(dataset, "\\/*?\"<>| ,#:") {
Copy link
Contributor

Choose a reason for hiding this comment

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

See above.

It looks like all 3 methods are identical besides the length limit?

return false
}
return true
}
79 changes: 27 additions & 52 deletions x-pack/elastic-agent/pkg/agent/application/filters/stream_checker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,25 +93,6 @@ func TestStreamCheck(t *testing.T) {
},
result: ErrInvalidDataset,
},

{
name: "dataset invalid dot - compact",
configMap: map[string]interface{}{
"inputs": []map[string]interface{}{
{"streams": []map[string]interface{}{{"data_stream.dataset": "."}}},
},
},
result: ErrInvalidDataset,
},
{
name: "dataset invalid dotdot- compact",
configMap: map[string]interface{}{
"inputs": []map[string]interface{}{
{"streams": []map[string]interface{}{{"data_stream.dataset": ".."}}},
},
},
result: ErrInvalidDataset,
},
{
name: "dataset invalid uppercase - compact",
configMap: map[string]interface{}{
Expand Down Expand Up @@ -139,39 +120,13 @@ func TestStreamCheck(t *testing.T) {
},
result: ErrInvalidDataset,
},
{
name: "dataset invalid invalid prefix- compact",
configMap: map[string]interface{}{
"inputs": []map[string]interface{}{
{"streams": []map[string]interface{}{{"data_stream.dataset": "_isthisvalid"}}},
},
},
result: ErrInvalidDataset,
},

{
name: "namespace invalid - compact",
configMap: map[string]interface{}{
"inputs": []map[string]interface{}{{"data_stream.namespace": ""}},
},
result: ErrInvalidNamespace,
},
{
name: "namespace invalid name 1 - compact",
configMap: map[string]interface{}{
"inputs": []map[string]interface{}{
{"data_stream.namespace": "."},
},
},
result: ErrInvalidNamespace,
},
{
name: "namespace invalid name 2 - compact",
configMap: map[string]interface{}{
"inputs": []map[string]interface{}{{"data_stream.namespace": ".."}},
},
result: ErrInvalidNamespace,
},
{
name: "namespace invalid name uppercase - compact",
configMap: map[string]interface{}{
Expand All @@ -193,13 +148,6 @@ func TestStreamCheck(t *testing.T) {
},
result: ErrInvalidNamespace,
},
{
name: "namespace invalid name invalid prefix - compact",
configMap: map[string]interface{}{
"inputs": []map[string]interface{}{{"data_stream.namespace": "+isitok"}},
},
result: ErrInvalidNamespace,
},
{
name: "namespace invalid - long",
configMap: map[string]interface{}{
Expand Down Expand Up @@ -274,6 +222,33 @@ func TestStreamCheck(t *testing.T) {
},
result: nil,
},
{
name: "type invalid prefix _",
configMap: map[string]interface{}{
"inputs": []map[string]interface{}{
{"data_stream.type": "_type"},
},
},
result: ErrInvalidIndex,
},
{
name: "type invalid prefix -",
configMap: map[string]interface{}{
"inputs": []map[string]interface{}{
{"data_stream.type": "-type"},
},
},
result: ErrInvalidIndex,
},
{
name: "type invalid prefix +",
configMap: map[string]interface{}{
"inputs": []map[string]interface{}{
{"data_stream.type": "+type"},
},
},
result: ErrInvalidIndex,
},
}

log, err := logger.New("")
Expand Down