Skip to content

Commit

Permalink
feat(pkger): enforce metadata.name dns name compliance
Browse files Browse the repository at this point in the history
closes: #17940
  • Loading branch information
jsteenb2 committed May 3, 2020
1 parent 35ed573 commit 9af4d50
Show file tree
Hide file tree
Showing 18 changed files with 148 additions and 84 deletions.
35 changes: 18 additions & 17 deletions cmd/influxd/launcher/pkger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ func TestLauncher_Pkger(t *testing.T) {
initialRulePkgName = "oh_doyle_rules"
initialTaskPkgName = "tap"
initialTelegrafPkgName = "teletype"
initialVariablePkgName = "laces out dan"
initialVariablePkgName = "laces_out_dan"
)
initialPkg := newPkg(
newBucketObject(initialBucketPkgName, "display name", "init desc"),
Expand Down Expand Up @@ -1837,8 +1837,9 @@ spec:
apiVersion: %[1]s
kind: Task
metadata:
name: Http.POST Synthetic (POST)
name: http_post_synthetic
spec:
name: Http.POST Synthetic (POST)
every: 5m
query: |-
import "strings"
Expand Down Expand Up @@ -1992,24 +1993,24 @@ spec:
require.NoError(t, err)

require.Len(t, sum.Buckets, 1)
assert.Equal(t, "$bkt-1-name-ref", sum.Buckets[0].Name)
assert.Equal(t, "env_bkt-1-name-ref", sum.Buckets[0].Name)
assert.Len(t, sum.Buckets[0].LabelAssociations, 1)
require.Len(t, sum.Checks, 1)
assert.Equal(t, "$check-1-name-ref", sum.Checks[0].Check.GetName())
assert.Equal(t, "env_check-1-name-ref", sum.Checks[0].Check.GetName())
require.Len(t, sum.Dashboards, 1)
assert.Equal(t, "$dash-1-name-ref", sum.Dashboards[0].Name)
assert.Equal(t, "env_dash-1-name-ref", sum.Dashboards[0].Name)
require.Len(t, sum.Labels, 1)
assert.Equal(t, "$label-1-name-ref", sum.Labels[0].Name)
assert.Equal(t, "env_label-1-name-ref", sum.Labels[0].Name)
require.Len(t, sum.NotificationEndpoints, 1)
assert.Equal(t, "$endpoint-1-name-ref", sum.NotificationEndpoints[0].NotificationEndpoint.GetName())
assert.Equal(t, "env_endpoint-1-name-ref", sum.NotificationEndpoints[0].NotificationEndpoint.GetName())
require.Len(t, sum.NotificationRules, 1)
assert.Equal(t, "$rule-1-name-ref", sum.NotificationRules[0].Name)
assert.Equal(t, "env_rule-1-name-ref", sum.NotificationRules[0].Name)
require.Len(t, sum.TelegrafConfigs, 1)
assert.Equal(t, "$task-1-name-ref", sum.Tasks[0].Name)
assert.Equal(t, "env_task-1-name-ref", sum.Tasks[0].Name)
require.Len(t, sum.TelegrafConfigs, 1)
assert.Equal(t, "$telegraf-1-name-ref", sum.TelegrafConfigs[0].TelegrafConfig.Name)
assert.Equal(t, "env_telegraf-1-name-ref", sum.TelegrafConfigs[0].TelegrafConfig.Name)
require.Len(t, sum.Variables, 1)
assert.Equal(t, "$var-1-name-ref", sum.Variables[0].Name)
assert.Equal(t, "env_var-1-name-ref", sum.Variables[0].Name)

expectedMissingEnvs := []string{
"bkt-1-name-ref",
Expand Down Expand Up @@ -2083,7 +2084,7 @@ metadata:
apiVersion: %[1]s
kind: Label
metadata:
name: the 2nd label
name: the_2nd_label
spec:
name: the 2nd label
---
Expand All @@ -2097,12 +2098,12 @@ spec:
- kind: Label
name: label_1
- kind: Label
name: the 2nd label
name: the_2nd_label
---
apiVersion: %[1]s
kind: Dashboard
metadata:
name: dash_UUID
name: dash_uuid
spec:
name: dash_1
description: desc1
Expand All @@ -2124,7 +2125,7 @@ spec:
- kind: Label
name: label_1
- kind: Label
name: the 2nd label
name: the_2nd_label
---
apiVersion: %[1]s
kind: Variable
Expand Down Expand Up @@ -2232,7 +2233,7 @@ spec:
apiVersion: %[1]s
kind: NotificationRule
metadata:
name: rule_UUID
name: rule_uuid
spec:
name: rule_0
description: desc_0
Expand All @@ -2259,7 +2260,7 @@ spec:
apiVersion: %[1]s
kind: Task
metadata:
name: task_UUID
name: task_uuid
spec:
name: task_1
description: desc_1
Expand Down
2 changes: 1 addition & 1 deletion pkger/models_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ func TestPkg(t *testing.T) {
{
pkgFile: "testdata/notification_rule.yml",
kind: KindNotificationRule,
validName: "rule_UUID",
validName: "rule_uuid",
},
}

Expand Down
72 changes: 60 additions & 12 deletions pkger/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ func (p *Pkg) graphResources() error {
func (p *Pkg) graphBuckets() *parseErr {
p.mBuckets = make(map[string]*bucket)
tracker := p.trackNames(true)
return p.eachResource(KindBucket, bucketNameMinLength, func(o Object) []validationErr {
return p.eachResource(KindBucket, func(o Object) []validationErr {
ident, errs := tracker(o)
if len(errs) > 0 {
return errs
Expand Down Expand Up @@ -770,7 +770,7 @@ func (p *Pkg) graphBuckets() *parseErr {
func (p *Pkg) graphLabels() *parseErr {
p.mLabels = make(map[string]*label)
tracker := p.trackNames(true)
return p.eachResource(KindLabel, labelNameMinLength, func(o Object) []validationErr {
return p.eachResource(KindLabel, func(o Object) []validationErr {
ident, errs := tracker(o)
if len(errs) > 0 {
return errs
Expand Down Expand Up @@ -801,7 +801,7 @@ func (p *Pkg) graphChecks() *parseErr {
}
var pErr parseErr
for _, checkKind := range checkKinds {
err := p.eachResource(checkKind.kind, checkNameMinLength, func(o Object) []validationErr {
err := p.eachResource(checkKind.kind, func(o Object) []validationErr {
ident, errs := tracker(o)
if len(errs) > 0 {
return errs
Expand Down Expand Up @@ -862,7 +862,7 @@ func (p *Pkg) graphChecks() *parseErr {
func (p *Pkg) graphDashboards() *parseErr {
p.mDashboards = make(map[string]*dashboard)
tracker := p.trackNames(false)
return p.eachResource(KindDashboard, dashboardNameMinLength, func(o Object) []validationErr {
return p.eachResource(KindDashboard, func(o Object) []validationErr {
ident, errs := tracker(o)
if len(errs) > 0 {
return errs
Expand Down Expand Up @@ -926,7 +926,7 @@ func (p *Pkg) graphNotificationEndpoints() *parseErr {

var pErr parseErr
for _, nk := range notificationKinds {
err := p.eachResource(nk.kind, 1, func(o Object) []validationErr {
err := p.eachResource(nk.kind, func(o Object) []validationErr {
ident, errs := tracker(o)
if len(errs) > 0 {
return errs
Expand Down Expand Up @@ -977,7 +977,7 @@ func (p *Pkg) graphNotificationEndpoints() *parseErr {
func (p *Pkg) graphNotificationRules() *parseErr {
p.mNotificationRules = make(map[string]*notificationRule)
tracker := p.trackNames(false)
return p.eachResource(KindNotificationRule, 1, func(o Object) []validationErr {
return p.eachResource(KindNotificationRule, func(o Object) []validationErr {
ident, errs := tracker(o)
if len(errs) > 0 {
return errs
Expand Down Expand Up @@ -1027,7 +1027,7 @@ func (p *Pkg) graphNotificationRules() *parseErr {
func (p *Pkg) graphTasks() *parseErr {
p.mTasks = make(map[string]*task)
tracker := p.trackNames(false)
return p.eachResource(KindTask, 1, func(o Object) []validationErr {
return p.eachResource(KindTask, func(o Object) []validationErr {
ident, errs := tracker(o)
if len(errs) > 0 {
return errs
Expand Down Expand Up @@ -1059,7 +1059,7 @@ func (p *Pkg) graphTasks() *parseErr {
func (p *Pkg) graphTelegrafs() *parseErr {
p.mTelegrafs = make(map[string]*telegraf)
tracker := p.trackNames(false)
return p.eachResource(KindTelegraf, 0, func(o Object) []validationErr {
return p.eachResource(KindTelegraf, func(o Object) []validationErr {
ident, errs := tracker(o)
if len(errs) > 0 {
return errs
Expand Down Expand Up @@ -1088,7 +1088,7 @@ func (p *Pkg) graphTelegrafs() *parseErr {
func (p *Pkg) graphVariables() *parseErr {
p.mVariables = make(map[string]*variable)
tracker := p.trackNames(true)
return p.eachResource(KindVariable, 1, func(o Object) []validationErr {
return p.eachResource(KindVariable, func(o Object) []validationErr {
ident, errs := tracker(o)
if len(errs) > 0 {
return errs
Expand Down Expand Up @@ -1118,7 +1118,7 @@ func (p *Pkg) graphVariables() *parseErr {
})
}

func (p *Pkg) eachResource(resourceKind Kind, minNameLen int, fn func(o Object) []validationErr) *parseErr {
func (p *Pkg) eachResource(resourceKind Kind, fn func(o Object) []validationErr) *parseErr {
var pErr parseErr
for i, k := range p.Objects {
if err := k.Kind.OK(); err != nil {
Expand Down Expand Up @@ -1152,14 +1152,14 @@ func (p *Pkg) eachResource(resourceKind Kind, minNameLen int, fn func(o Object)
continue
}

if len(k.Name()) < minNameLen {
if !isDomainName(k.Name()) {
pErr.append(resourceErr{
Kind: k.Kind.String(),
Idx: intPtr(i),
ValidationErrs: []validationErr{
objectValidationErr(fieldMetadata, validationErr{
Field: fieldName,
Msg: fmt.Sprintf("must be a string of at least %d chars in length", minNameLen),
Msg: "must be a between 3-64 characters long and may only consist of alphanumeric, dots, underderscores, and hyphen characters.",
}),
},
})
Expand Down Expand Up @@ -1434,6 +1434,54 @@ func parseChart(r Resource) (chart, []validationErr) {
return c, nil
}

// isDomainName is composed of 98% of the std lib implementation, only
// difference in ours is that we only allow lower case letters.
func isDomainName(s string) bool {
l := len(s)
if l == 0 || l > 254 || l == 254 && s[l-1] != '.' {
return false
}

last := byte('.')
nonNumeric := false // true once we've seen a letter or hyphen
partlen := 0
for i := 0; i < len(s); i++ {
c := s[i]
switch {
default:
return false
case 'a' <= c && c <= 'z' || c == '_':
nonNumeric = true
partlen++
case '0' <= c && c <= '9':
// fine
partlen++
case c == '-':
// Byte before dash cannot be dot.
if last == '.' {
return false
}
partlen++
nonNumeric = true
case c == '.':
// Byte before dot cannot be dot, dash.
if last == '.' || last == '-' {
return false
}
if partlen > 63 || partlen == 0 {
return false
}
partlen = 0
}
last = c
}
if last == '-' || partlen > 63 {
return false
}

return nonNumeric
}

// Resource is a pkger Resource kind. It can be one of any of
// available kinds that are supported.
type Resource map[string]interface{}
Expand Down
19 changes: 18 additions & 1 deletion pkger/parser_models.go
Original file line number Diff line number Diff line change
Expand Up @@ -1335,6 +1335,10 @@ var validEndpointHTTPMethods = map[string]bool{

func (n *notificationEndpoint) valid() []validationErr {
var failures []validationErr
if err, ok := isValidName(n.Name(), 1); !ok {
failures = append(failures, err)
}

if _, err := url.Parse(n.url); err != nil || n.url == "" {
failures = append(failures, validationErr{
Field: fieldNotificationEndpointURL,
Expand Down Expand Up @@ -1532,6 +1536,9 @@ func (r *notificationRule) toInfluxRule() influxdb.NotificationRule {

func (r *notificationRule) valid() []validationErr {
var vErrs []validationErr
if err, ok := isValidName(r.Name(), 1); !ok {
vErrs = append(vErrs, err)
}
if !r.endpointName.hasValue() {
vErrs = append(vErrs, validationErr{
Field: fieldNotificationRuleEndpointName,
Expand Down Expand Up @@ -1714,6 +1721,9 @@ func (t *task) summarize() SummaryTask {

func (t *task) valid() []validationErr {
var vErrs []validationErr
if err, ok := isValidName(t.Name(), 1); !ok {
vErrs = append(vErrs, err)
}
if t.cron == "" && t.every == 0 {
vErrs = append(vErrs,
validationErr{
Expand Down Expand Up @@ -1838,6 +1848,9 @@ func (t *telegraf) summarize() SummaryTelegraf {

func (t *telegraf) valid() []validationErr {
var vErrs []validationErr
if err, ok := isValidName(t.Name(), 1); !ok {
vErrs = append(vErrs, err)
}
if t.config.Config == "" {
vErrs = append(vErrs, validationErr{
Field: fieldTelegrafConfig,
Expand Down Expand Up @@ -1918,6 +1931,10 @@ func (v *variable) influxVarArgs() *influxdb.VariableArguments {

func (v *variable) valid() []validationErr {
var failures []validationErr
if err, ok := isValidName(v.Name(), 1); !ok {
failures = append(failures, err)
}

switch v.Type {
case "map":
if len(v.MapValues) == 0 {
Expand Down Expand Up @@ -1979,7 +1996,7 @@ func (r *references) String() string {
return v
}
if r.EnvRef != "" {
return "$" + r.EnvRef
return "env_" + r.EnvRef
}
return ""
}
Expand Down
Loading

0 comments on commit 9af4d50

Please sign in to comment.