From 082b45478d18381f959ec578846a19366344268e Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Wed, 8 Apr 2020 22:14:06 -0400 Subject: [PATCH 1/2] spec: unify resource fetching type Dedupe the "remote resource" type from the spec. This came up too during review of the new HTTP headers. They all had more or less the same fields in common ("source", "verification", "compression", and more recently "httpHeaders"). One noticeable spec level change from this is that some places now support compression where they didn't before. Might seem odd to support compression for e.g. certificate authorities, though it does make the spec more consistent throughout. Co-authored-by: Jonathan Lebon --- config/merge/merge_test.go | 44 ++++----- config/shared/errors/errors.go | 1 + config/v3_1_experimental/schema/ignition.json | 78 +++++----------- .../v3_1_experimental/translate/translate.go | 8 +- config/v3_1_experimental/types/ca.go | 56 ------------ config/v3_1_experimental/types/file.go | 51 ----------- config/v3_1_experimental/types/file_test.go | 14 +-- config/v3_1_experimental/types/ignition.go | 43 ++------- .../v3_1_experimental/types/ignition_test.go | 48 ++++++++++ config/v3_1_experimental/types/resource.go | 91 +++++++++++++++++++ config/v3_1_experimental/types/schema.go | 38 +++----- config/v3_1_experimental/types/tls.go | 27 ++++++ config/v3_1_experimental/types/tls_test.go | 42 +++++++++ doc/configuration-v3_1-experimental.md | 3 + internal/exec/engine.go | 16 +++- internal/exec/util/file.go | 2 +- internal/resource/http.go | 31 +++++-- 17 files changed, 322 insertions(+), 271 deletions(-) delete mode 100644 config/v3_1_experimental/types/ca.go create mode 100644 config/v3_1_experimental/types/ignition_test.go create mode 100644 config/v3_1_experimental/types/resource.go create mode 100644 config/v3_1_experimental/types/tls.go create mode 100644 config/v3_1_experimental/types/tls_test.go diff --git a/config/merge/merge_test.go b/config/merge/merge_test.go index b1b12a95c..c12cc9134 100644 --- a/config/merge/merge_test.go +++ b/config/merge/merge_test.go @@ -82,7 +82,7 @@ func TestMerge(t *testing.T) { Path: "/foo", }, FileEmbedded1: types.FileEmbedded1{ - Append: []types.FileContents{ + Append: []types.Resource{ { Source: util.StrToPtr("source1"), }, @@ -94,7 +94,7 @@ func TestMerge(t *testing.T) { Path: "/bar", }, FileEmbedded1: types.FileEmbedded1{ - Contents: types.FileContents{ + Contents: types.Resource{ Compression: util.StrToPtr("gzip"), }, }, @@ -133,7 +133,7 @@ func TestMerge(t *testing.T) { Path: "/foo", }, FileEmbedded1: types.FileEmbedded1{ - Append: []types.FileContents{ + Append: []types.Resource{ { Source: util.StrToPtr("source1"), }, @@ -193,7 +193,7 @@ func TestMerge(t *testing.T) { Path: "/foo", }, FileEmbedded1: types.FileEmbedded1{ - Append: []types.FileContents{ + Append: []types.Resource{ { Source: util.StrToPtr("source1"), }, @@ -230,7 +230,7 @@ func TestMerge(t *testing.T) { in1: types.Config{ Ignition: types.Ignition{ Config: types.IgnitionConfig{ - Merge: []types.ConfigReference{ + Merge: []types.Resource{ { Source: &configURL, HTTPHeaders: []types.HTTPHeader{ @@ -255,7 +255,7 @@ func TestMerge(t *testing.T) { in2: types.Config{ Ignition: types.Ignition{ Config: types.IgnitionConfig{ - Merge: []types.ConfigReference{ + Merge: []types.Resource{ { Source: &configURL, HTTPHeaders: []types.HTTPHeader{ @@ -279,7 +279,7 @@ func TestMerge(t *testing.T) { out: types.Config{ Ignition: types.Ignition{ Config: types.IgnitionConfig{ - Merge: []types.ConfigReference{ + Merge: []types.Resource{ { Source: &configURL, HTTPHeaders: []types.HTTPHeader{ @@ -308,7 +308,7 @@ func TestMerge(t *testing.T) { in1: types.Config{ Ignition: types.Ignition{ Config: types.IgnitionConfig{ - Replace: types.ConfigReference{ + Replace: types.Resource{ Source: &configURL, HTTPHeaders: []types.HTTPHeader{ { @@ -331,7 +331,7 @@ func TestMerge(t *testing.T) { in2: types.Config{ Ignition: types.Ignition{ Config: types.IgnitionConfig{ - Replace: types.ConfigReference{ + Replace: types.Resource{ Source: &configURL, HTTPHeaders: []types.HTTPHeader{ { @@ -353,7 +353,7 @@ func TestMerge(t *testing.T) { out: types.Config{ Ignition: types.Ignition{ Config: types.IgnitionConfig{ - Replace: types.ConfigReference{ + Replace: types.Resource{ Source: &configURL, HTTPHeaders: []types.HTTPHeader{ { @@ -381,9 +381,9 @@ func TestMerge(t *testing.T) { Ignition: types.Ignition{ Security: types.Security{ TLS: types.TLS{ - CertificateAuthorities: []types.CaReference{ + CertificateAuthorities: []types.Resource{ { - Source: caURL, + Source: util.StrToPtr(caURL), HTTPHeaders: []types.HTTPHeader{ { Name: "old-header", @@ -408,9 +408,9 @@ func TestMerge(t *testing.T) { Ignition: types.Ignition{ Security: types.Security{ TLS: types.TLS{ - CertificateAuthorities: []types.CaReference{ + CertificateAuthorities: []types.Resource{ { - Source: caURL, + Source: util.StrToPtr(caURL), HTTPHeaders: []types.HTTPHeader{ { Name: "to-remove-header", @@ -434,9 +434,9 @@ func TestMerge(t *testing.T) { Ignition: types.Ignition{ Security: types.Security{ TLS: types.TLS{ - CertificateAuthorities: []types.CaReference{ + CertificateAuthorities: []types.Resource{ { - Source: caURL, + Source: util.StrToPtr(caURL), HTTPHeaders: []types.HTTPHeader{ { Name: "old-header", @@ -466,7 +466,7 @@ func TestMerge(t *testing.T) { Files: []types.File{ { FileEmbedded1: types.FileEmbedded1{ - Contents: types.FileContents{ + Contents: types.Resource{ Source: &fileURL, HTTPHeaders: []types.HTTPHeader{ { @@ -493,7 +493,7 @@ func TestMerge(t *testing.T) { Files: []types.File{ { FileEmbedded1: types.FileEmbedded1{ - Contents: types.FileContents{ + Contents: types.Resource{ Source: &fileURL, HTTPHeaders: []types.HTTPHeader{ { @@ -519,7 +519,7 @@ func TestMerge(t *testing.T) { Files: []types.File{ { FileEmbedded1: types.FileEmbedded1{ - Contents: types.FileContents{ + Contents: types.Resource{ Source: &fileURL, HTTPHeaders: []types.HTTPHeader{ { @@ -550,7 +550,7 @@ func TestMerge(t *testing.T) { Files: []types.File{ { FileEmbedded1: types.FileEmbedded1{ - Append: []types.FileContents{ + Append: []types.Resource{ { Source: &fileURL, HTTPHeaders: []types.HTTPHeader{ @@ -575,7 +575,7 @@ func TestMerge(t *testing.T) { Files: []types.File{ { FileEmbedded1: types.FileEmbedded1{ - Append: []types.FileContents{ + Append: []types.Resource{ { Source: &fileURL, HTTPHeaders: []types.HTTPHeader{ @@ -600,7 +600,7 @@ func TestMerge(t *testing.T) { Files: []types.File{ { FileEmbedded1: types.FileEmbedded1{ - Append: []types.FileContents{ + Append: []types.Resource{ { Source: &fileURL, HTTPHeaders: []types.HTTPHeader{ diff --git a/config/shared/errors/errors.go b/config/shared/errors/errors.go index c6a515f13..f84b444fe 100644 --- a/config/shared/errors/errors.go +++ b/config/shared/errors/errors.go @@ -78,6 +78,7 @@ var ( ErrInvalidInstantiatedUnit = errors.New("invalid systemd instantiated unit") // Misc errors + ErrSourceRequired = errors.New("source is required") ErrInvalidScheme = errors.New("invalid url scheme") ErrInvalidUrl = errors.New("unable to parse url") ErrInvalidHTTPHeader = errors.New("unable to parse HTTP header") diff --git a/config/v3_1_experimental/schema/ignition.json b/config/v3_1_experimental/schema/ignition.json index 4b7720c64..495b040f8 100644 --- a/config/v3_1_experimental/schema/ignition.json +++ b/config/v3_1_experimental/schema/ignition.json @@ -20,6 +20,23 @@ "ignition" ], "definitions": { + "resource": { + "type": "object", + "properties": { + "source": { + "type": ["string", "null"] + }, + "compression": { + "type": ["string", "null"] + }, + "httpHeaders": { + "$ref": "#/definitions/httpHeaders" + }, + "verification": { + "$ref": "#/definitions/verification" + } + } + }, "verification": { "type": "object", "properties": { @@ -69,11 +86,11 @@ "merge": { "type": "array", "items": { - "$ref": "#/definitions/ignition/definitions/config-reference" + "$ref": "#/definitions/resource" } }, "replace": { - "$ref": "#/definitions/ignition/definitions/config-reference" + "$ref": "#/definitions/resource" } } }, @@ -86,7 +103,7 @@ "certificateAuthorities": { "type": "array", "items": { - "$ref": "#/definitions/ignition/definitions/ca-reference" + "$ref": "#/definitions/resource" } } } @@ -110,40 +127,6 @@ } } }, - "config-reference": { - "type": "object", - "properties": { - "source": { - "type": ["string", "null"] - }, - "httpHeaders": { - "$ref": "#/definitions/httpHeaders" - }, - "verification": { - "$ref": "#/definitions/verification" - } - }, - "required": [ - "source" - ] - }, - "ca-reference": { - "type": ["object", "null"], - "properties": { - "source": { - "type": "string" - }, - "httpHeaders": { - "$ref": "#/definitions/httpHeaders" - }, - "verification": { - "$ref": "#/definitions/verification" - } - }, - "required": [ - "source" - ] - }, "timeouts": { "type": "object", "properties": { @@ -299,12 +282,12 @@ "type": ["integer", "null"] }, "contents": { - "$ref": "#/definitions/storage/definitions/file-contents" + "$ref": "#/definitions/resource" }, "append": { "type": "array", "items": { - "$ref": "#/definitions/storage/definitions/file-contents" + "$ref": "#/definitions/resource" } } } @@ -376,23 +359,6 @@ } } }, - "file-contents": { - "type": "object", - "properties": { - "compression": { - "type": ["string", "null"] - }, - "source": { - "type": ["string", "null"] - }, - "httpHeaders": { - "$ref": "#/definitions/httpHeaders" - }, - "verification": { - "$ref": "#/definitions/verification" - } - } - }, "node": { "type": "object", "properties": { diff --git a/config/v3_1_experimental/translate/translate.go b/config/v3_1_experimental/translate/translate.go index e42eb49ea..a672f3a1a 100644 --- a/config/v3_1_experimental/translate/translate.go +++ b/config/v3_1_experimental/translate/translate.go @@ -33,7 +33,7 @@ func translateFilesystem(old old_types.Filesystem) (ret types.Filesystem) { return } -func translateConfigReference(old old_types.ConfigReference) (ret types.ConfigReference) { +func translateConfigReference(old old_types.ConfigReference) (ret types.Resource) { // use a new translator so we don't recurse infinitely tr := translate.NewTranslator() tr.Translate(&old.Source, &ret.Source) @@ -41,15 +41,15 @@ func translateConfigReference(old old_types.ConfigReference) (ret types.ConfigRe return } -func translateCAReference(old old_types.CaReference) (ret types.CaReference) { +func translateCAReference(old old_types.CaReference) (ret types.Resource) { // use a new translator so we don't recurse infinitely tr := translate.NewTranslator() - tr.Translate(&old.Source, &ret.Source) + ret.Source = &old.Source tr.Translate(&old.Verification, &ret.Verification) return } -func translateFileContents(old old_types.FileContents) (ret types.FileContents) { +func translateFileContents(old old_types.FileContents) (ret types.Resource) { // use a new translator so we don't recurse infinitely tr := translate.NewTranslator() tr.Translate(&old.Compression, &ret.Compression) diff --git a/config/v3_1_experimental/types/ca.go b/config/v3_1_experimental/types/ca.go deleted file mode 100644 index 8c3f43797..000000000 --- a/config/v3_1_experimental/types/ca.go +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2018 CoreOS, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package types - -import ( - "net/url" - - "github.com/coreos/vcontext/path" - "github.com/coreos/vcontext/report" - - "github.com/coreos/ignition/v2/config/shared/errors" -) - -func (c CaReference) Key() string { - return c.Source -} - -func (ca CaReference) Validate(c path.ContextPath) (r report.Report) { - r.AddOnError(c.Append("source"), validateURL(ca.Source)) - r.AddOnError(c.Append("httpHeaders"), ca.validateSchemeForHTTPHeaders()) - return -} - -func (ca CaReference) validateSchemeForHTTPHeaders() error { - if len(ca.HTTPHeaders) < 1 { - return nil - } - - if ca.Source == "" { - return errors.ErrInvalidUrl - } - - u, err := url.Parse(ca.Source) - if err != nil { - return errors.ErrInvalidUrl - } - - switch u.Scheme { - case "http", "https": - return nil - default: - return errors.ErrUnsupportedSchemeForHTTPHeaders - } -} diff --git a/config/v3_1_experimental/types/file.go b/config/v3_1_experimental/types/file.go index 33614fe4b..d091c6705 100644 --- a/config/v3_1_experimental/types/file.go +++ b/config/v3_1_experimental/types/file.go @@ -15,10 +15,7 @@ package types import ( - "net/url" - "github.com/coreos/ignition/v2/config/shared/errors" - "github.com/coreos/ignition/v2/config/util" "github.com/coreos/vcontext/path" "github.com/coreos/vcontext/report" @@ -46,51 +43,3 @@ func (f FileEmbedded1) IgnoreDuplicates() map[string]struct{} { "Append": {}, } } - -func (fc FileContents) Validate(c path.ContextPath) (r report.Report) { - r.AddOnError(c.Append("compression"), fc.validateCompression()) - r.AddOnError(c.Append("verification", "hash"), fc.validateVerification()) - r.AddOnError(c.Append("source"), validateURLNilOK(fc.Source)) - r.AddOnError(c.Append("httpHeaders"), fc.validateSchemeForHTTPHeaders()) - return -} - -func (fc FileContents) validateCompression() error { - if fc.Compression != nil { - switch *fc.Compression { - case "", "gzip": - default: - return errors.ErrCompressionInvalid - } - } - return nil -} - -func (fc FileContents) validateVerification() error { - if fc.Verification.Hash != nil && fc.Source == nil { - return errors.ErrVerificationAndNilSource - } - return nil -} - -func (fc FileContents) validateSchemeForHTTPHeaders() error { - if len(fc.HTTPHeaders) < 1 { - return nil - } - - if util.NilOrEmpty(fc.Source) { - return errors.ErrInvalidUrl - } - - u, err := url.Parse(*fc.Source) - if err != nil { - return errors.ErrInvalidUrl - } - - switch u.Scheme { - case "http", "https": - return nil - default: - return errors.ErrUnsupportedSchemeForHTTPHeaders - } -} diff --git a/config/v3_1_experimental/types/file_test.go b/config/v3_1_experimental/types/file_test.go index ca0e3582d..f44bf890d 100644 --- a/config/v3_1_experimental/types/file_test.go +++ b/config/v3_1_experimental/types/file_test.go @@ -44,7 +44,7 @@ func TestFileValidateOverwrite(t *testing.T) { Overwrite: util.BoolToPtr(true), }, FileEmbedded1: FileEmbedded1{ - Contents: FileContents{ + Contents: Resource{ Source: util.StrToPtr(""), }, }, @@ -57,7 +57,7 @@ func TestFileValidateOverwrite(t *testing.T) { Overwrite: util.BoolToPtr(true), }, FileEmbedded1: FileEmbedded1{ - Contents: FileContents{ + Contents: Resource{ Source: util.StrToPtr("http://example.com"), }, }, @@ -76,21 +76,21 @@ func TestFileValidateOverwrite(t *testing.T) { func TestFileContentsValidate(t *testing.T) { tests := []struct { - in FileContents + in Resource out error }{ { - FileContents{}, + Resource{}, nil, }, { - FileContents{ + Resource{ Source: util.StrToPtr(""), }, nil, }, { - FileContents{ + Resource{ Source: util.StrToPtr(""), Verification: Verification{ Hash: util.StrToPtr(""), @@ -99,7 +99,7 @@ func TestFileContentsValidate(t *testing.T) { nil, }, { - FileContents{ + Resource{ Verification: Verification{ Hash: util.StrToPtr(""), }, diff --git a/config/v3_1_experimental/types/ignition.go b/config/v3_1_experimental/types/ignition.go index eac67b035..689b6093f 100644 --- a/config/v3_1_experimental/types/ignition.go +++ b/config/v3_1_experimental/types/ignition.go @@ -15,54 +15,23 @@ package types import ( - "net/url" - "github.com/coreos/go-semver/semver" "github.com/coreos/ignition/v2/config/shared/errors" - "github.com/coreos/ignition/v2/config/util" "github.com/coreos/vcontext/path" "github.com/coreos/vcontext/report" ) -func (c ConfigReference) Key() string { - if c.Source == nil { - return "" - } - return *c.Source -} - -func (cr ConfigReference) Validate(c path.ContextPath) (r report.Report) { - r.AddOnError(c.Append("source"), validateURLNilOK(cr.Source)) - r.AddOnError(c.Append("httpHeaders"), cr.validateSchemeForHTTPHeaders()) - return +func (v Ignition) Semver() (*semver.Version, error) { + return semver.NewVersion(v.Version) } -func (cr ConfigReference) validateSchemeForHTTPHeaders() error { - if len(cr.HTTPHeaders) < 1 { - return nil +func (ic IgnitionConfig) Validate(c path.ContextPath) (r report.Report) { + for i, res := range ic.Merge { + r.AddOnError(c.Append("merge", i), res.validateRequiredSource()) } - - if util.NilOrEmpty(cr.Source) { - return errors.ErrInvalidUrl - } - - u, err := url.Parse(*cr.Source) - if err != nil { - return errors.ErrInvalidUrl - } - - switch u.Scheme { - case "http", "https": - return nil - default: - return errors.ErrUnsupportedSchemeForHTTPHeaders - } -} - -func (v Ignition) Semver() (*semver.Version, error) { - return semver.NewVersion(v.Version) + return } func (v Ignition) Validate(c path.ContextPath) (r report.Report) { diff --git a/config/v3_1_experimental/types/ignition_test.go b/config/v3_1_experimental/types/ignition_test.go new file mode 100644 index 000000000..ac7d462d3 --- /dev/null +++ b/config/v3_1_experimental/types/ignition_test.go @@ -0,0 +1,48 @@ +// Copyright 2020 Red Hat, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package types + +import ( + "testing" + + "github.com/coreos/vcontext/validate" +) + +func TestIgnitionConfigValidate(t *testing.T) { + tests := []struct { + in IgnitionConfig + out string + }{ + { + IgnitionConfig{ + Merge: []Resource{{}}, + }, + "error at $.merge.0: source is required\n", + }, + { + IgnitionConfig{ + Replace: Resource{}, + }, + "", + }, + } + + for i, test := range tests { + r := validate.Validate(test.in, "test") + if test.out != r.String() { + t.Errorf("#%d: bad error: want %q, got %q", i, test.out, r.String()) + } + } +} diff --git a/config/v3_1_experimental/types/resource.go b/config/v3_1_experimental/types/resource.go new file mode 100644 index 000000000..68da6c7b7 --- /dev/null +++ b/config/v3_1_experimental/types/resource.go @@ -0,0 +1,91 @@ +// Copyright 2020 Red Hat, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package types + +import ( + "net/url" + + "github.com/coreos/ignition/v2/config/shared/errors" + "github.com/coreos/ignition/v2/config/util" + + "github.com/coreos/vcontext/path" + "github.com/coreos/vcontext/report" +) + +func (res Resource) Key() string { + if res.Source == nil { + return "" + } + return *res.Source +} + +func (res Resource) Validate(c path.ContextPath) (r report.Report) { + r.AddOnError(c.Append("compression"), res.validateCompression()) + r.AddOnError(c.Append("verification", "hash"), res.validateVerification()) + r.AddOnError(c.Append("source"), validateURLNilOK(res.Source)) + r.AddOnError(c.Append("httpHeaders"), res.validateSchemeForHTTPHeaders()) + return +} + +func (res Resource) validateCompression() error { + if res.Compression != nil { + switch *res.Compression { + case "", "gzip": + default: + return errors.ErrCompressionInvalid + } + } + return nil +} + +func (res Resource) validateVerification() error { + if res.Verification.Hash != nil && res.Source == nil { + return errors.ErrVerificationAndNilSource + } + return nil +} + +func (res Resource) validateSchemeForHTTPHeaders() error { + if len(res.HTTPHeaders) < 1 { + return nil + } + + if util.NilOrEmpty(res.Source) { + return errors.ErrInvalidUrl + } + + u, err := url.Parse(*res.Source) + if err != nil { + return errors.ErrInvalidUrl + } + + switch u.Scheme { + case "http", "https": + return nil + default: + return errors.ErrUnsupportedSchemeForHTTPHeaders + } +} + +// Ensure that the Source is specified and valid. This is not called by +// Resource.Validate() because some structs that embed Resource don't +// require Source to be specified. Containing structs that require Source +// should call this function from their Validate(). +func (res Resource) validateRequiredSource() error { + if util.NilOrEmpty(res.Source) { + return errors.ErrSourceRequired + } + return validateURL(*res.Source) +} diff --git a/config/v3_1_experimental/types/schema.go b/config/v3_1_experimental/types/schema.go index 309ae4ce6..c817e589b 100644 --- a/config/v3_1_experimental/types/schema.go +++ b/config/v3_1_experimental/types/schema.go @@ -2,12 +2,6 @@ package types // generated by "schematyper --package=types config/v3_1_experimental/schema/ignition.json -o config/v3_1_experimental/types/schema.go --root-type=Config" -- DO NOT EDIT -type CaReference struct { - HTTPHeaders HTTPHeaders `json:"httpHeaders,omitempty"` - Source string `json:"source"` - Verification Verification `json:"verification,omitempty"` -} - type Config struct { Ignition Ignition `json:"ignition"` Passwd Passwd `json:"passwd,omitempty"` @@ -15,12 +9,6 @@ type Config struct { Systemd Systemd `json:"systemd,omitempty"` } -type ConfigReference struct { - HTTPHeaders HTTPHeaders `json:"httpHeaders,omitempty"` - Source *string `json:"source"` - Verification Verification `json:"verification,omitempty"` -} - type Device string type Directory struct { @@ -48,17 +36,10 @@ type File struct { FileEmbedded1 } -type FileContents struct { - Compression *string `json:"compression,omitempty"` - HTTPHeaders HTTPHeaders `json:"httpHeaders,omitempty"` - Source *string `json:"source,omitempty"` - Verification Verification `json:"verification,omitempty"` -} - type FileEmbedded1 struct { - Append []FileContents `json:"append,omitempty"` - Contents FileContents `json:"contents,omitempty"` - Mode *int `json:"mode,omitempty"` + Append []Resource `json:"append,omitempty"` + Contents Resource `json:"contents,omitempty"` + Mode *int `json:"mode,omitempty"` } type Filesystem struct { @@ -92,8 +73,8 @@ type Ignition struct { } type IgnitionConfig struct { - Merge []ConfigReference `json:"merge,omitempty"` - Replace ConfigReference `json:"replace,omitempty"` + Merge []Resource `json:"merge,omitempty"` + Replace Resource `json:"replace,omitempty"` } type Link struct { @@ -182,6 +163,13 @@ type Raid struct { type RaidOption string +type Resource struct { + Compression *string `json:"compression,omitempty"` + HTTPHeaders HTTPHeaders `json:"httpHeaders,omitempty"` + Source *string `json:"source,omitempty"` + Verification Verification `json:"verification,omitempty"` +} + type SSHAuthorizedKey string type Security struct { @@ -202,7 +190,7 @@ type Systemd struct { } type TLS struct { - CertificateAuthorities []CaReference `json:"certificateAuthorities,omitempty"` + CertificateAuthorities []Resource `json:"certificateAuthorities,omitempty"` } type Timeouts struct { diff --git a/config/v3_1_experimental/types/tls.go b/config/v3_1_experimental/types/tls.go new file mode 100644 index 000000000..8890e397e --- /dev/null +++ b/config/v3_1_experimental/types/tls.go @@ -0,0 +1,27 @@ +// Copyright 2020 Red Hat, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package types + +import ( + "github.com/coreos/vcontext/path" + "github.com/coreos/vcontext/report" +) + +func (tls TLS) Validate(c path.ContextPath) (r report.Report) { + for i, ca := range tls.CertificateAuthorities { + r.AddOnError(c.Append("certificateAuthorities", i), ca.validateRequiredSource()) + } + return +} diff --git a/config/v3_1_experimental/types/tls_test.go b/config/v3_1_experimental/types/tls_test.go new file mode 100644 index 000000000..8f3b2e351 --- /dev/null +++ b/config/v3_1_experimental/types/tls_test.go @@ -0,0 +1,42 @@ +// Copyright 2020 Red Hat, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package types + +import ( + "testing" + + "github.com/coreos/vcontext/validate" +) + +func TestTLSValidate(t *testing.T) { + tests := []struct { + in TLS + out string + }{ + { + TLS{ + CertificateAuthorities: []Resource{{}}, + }, + "error at $.certificateAuthorities.0: source is required\n", + }, + } + + for i, test := range tests { + r := validate.Validate(test.in, "test") + if test.out != r.String() { + t.Errorf("#%d: bad error: want %q, got %q", i, test.out, r.String()) + } + } +} diff --git a/doc/configuration-v3_1-experimental.md b/doc/configuration-v3_1-experimental.md index a9ab1f82d..d6177d24a 100644 --- a/doc/configuration-v3_1-experimental.md +++ b/doc/configuration-v3_1-experimental.md @@ -9,6 +9,7 @@ The Ignition configuration is a JSON document conforming to the following specif * **_config_** (objects): options related to the configuration. * **_merge_** (list of objects): a list of the configs to be merged to the current config. * **source** (string): the URL of the config. Supported schemes are `http`, `https`, `s3`, `tftp`, and [`data`][rfc2397]. Note: When using `http`, it is advisable to use the verification option to ensure the contents haven't been modified. + * **_compression_** (string): the type of compression used on the config (null or gzip). Compression cannot be used with S3. * **_httpHeaders_** (list of objects): a list of HTTP headers to be added to the request. Available for `http` and `https` source schemes only. * **name** (string): the header name. * **_value_** (string): the header contents. @@ -16,6 +17,7 @@ The Ignition configuration is a JSON document conforming to the following specif * **_hash_** (string): the hash of the config, in the form `-` where type is `sha512`. * **_replace_** (object): the config that will replace the current. * **source** (string): the URL of the config. Supported schemes are `http`, `https`, `s3`, `tftp`, and [`data`][rfc2397]. Note: When using `http`, it is advisable to use the verification option to ensure the contents haven't been modified. + * **_compression_** (string): the type of compression used on the config (null or gzip). Compression cannot be used with S3. * **_httpHeaders_** (list of objects): a list of HTTP headers to be added to the request. Available for `http` and `https` source schemes only. * **name** (string): the header name. * **_value_** (string): the header contents. @@ -28,6 +30,7 @@ The Ignition configuration is a JSON document conforming to the following specif * **_tls_** (object): options relating to TLS when fetching resources over `https`. * **_certificateAuthorities_** (list of objects): the list of additional certificate authorities (in addition to the system authorities) to be used for TLS verification when fetching over `https`. All certificate authorities must have a unique `source`. * **source** (string): the URL of the certificate (in PEM format). Supported schemes are `http`, `https`, `s3`, `tftp`, and [`data`][rfc2397]. Note: When using `http`, it is advisable to use the verification option to ensure the contents haven't been modified. + * **_compression_** (string): the type of compression used on the certificate (null or gzip). Compression cannot be used with S3. * **_httpHeaders_** (list of objects): a list of HTTP headers to be added to the request. Available for `http` and `https` source schemes only. * **name** (string): the header name. * **_value_** (string): the header contents. diff --git a/internal/exec/engine.go b/internal/exec/engine.go index f9150780b..9db4f7212 100644 --- a/internal/exec/engine.go +++ b/internal/exec/engine.go @@ -267,8 +267,13 @@ func (e *Engine) renderConfig(cfg types.Config) (types.Config, error) { } // fetchReferencedConfig fetches and parses the requested config. -// cfgRef.Source must not ve nil -func (e *Engine) fetchReferencedConfig(cfgRef types.ConfigReference) (types.Config, error) { +// cfgRef.Source must not be nil +func (e *Engine) fetchReferencedConfig(cfgRef types.Resource) (types.Config, error) { + // this is also already checked at validation time + if cfgRef.Source == nil { + e.Logger.Crit("invalid referenced config: %v", errors.ErrSourceRequired) + return types.Config{}, errors.ErrSourceRequired + } u, err := url.Parse(*cfgRef.Source) if err != nil { return types.Config{}, err @@ -280,8 +285,13 @@ func (e *Engine) fetchReferencedConfig(cfgRef types.ConfigReference) (types.Conf return types.Config{}, err } } + compression := "" + if cfgRef.Compression != nil { + compression = *cfgRef.Compression + } rawCfg, err := e.Fetcher.FetchToBuffer(*u, resource.FetchOptions{ - Headers: headers, + Headers: headers, + Compression: compression, }) if err != nil { return types.Config{}, err diff --git a/internal/exec/util/file.go b/internal/exec/util/file.go index 78ca64b47..167560ae2 100644 --- a/internal/exec/util/file.go +++ b/internal/exec/util/file.go @@ -58,7 +58,7 @@ func newHashedReader(reader io.ReadCloser, hasher hash.Hash) io.ReadCloser { } } -func newFetchOp(l *log.Logger, node types.Node, contents types.FileContents) (FetchOp, error) { +func newFetchOp(l *log.Logger, node types.Node, contents types.Resource) (FetchOp, error) { var expectedSum []byte uri, err := url.Parse(*contents.Source) diff --git a/internal/resource/http.go b/internal/resource/http.go index bc3cbf9d6..0ff2512bc 100644 --- a/internal/resource/http.go +++ b/internal/resource/http.go @@ -28,6 +28,7 @@ import ( "strings" "time" + ignerrors "github.com/coreos/ignition/v2/config/shared/errors" "github.com/coreos/ignition/v2/config/v3_1_experimental/types" "github.com/coreos/ignition/v2/internal/earlyrand" "github.com/coreos/ignition/v2/internal/log" @@ -63,7 +64,7 @@ type HttpClient struct { cas map[string][]byte } -func (f *Fetcher) UpdateHttpTimeoutsAndCAs(timeouts types.Timeouts, cas []types.CaReference, proxy types.Proxy) error { +func (f *Fetcher) UpdateHttpTimeoutsAndCAs(timeouts types.Timeouts, cas []types.Resource, proxy types.Proxy) error { if f.client == nil { if err := f.newHttpClient(); err != nil { return err @@ -110,13 +111,13 @@ func (f *Fetcher) UpdateHttpTimeoutsAndCAs(timeouts types.Timeouts, cas []types. } block, _ := pem.Decode(cablob) if block == nil { - f.Logger.Err("Unable to decode CA (%s)", ca.Source) + f.Logger.Err("Unable to decode CA (%v)", ca.Source) return ErrPEMDecodeFailed } cert, err := x509.ParseCertificate(block.Bytes) if err != nil { - f.Logger.Err("Unable to parse CA (%s): %s", ca.Source, err) + f.Logger.Err("Unable to parse CA (%v): %s", ca.Source, err) return err } @@ -129,11 +130,16 @@ func (f *Fetcher) UpdateHttpTimeoutsAndCAs(timeouts types.Timeouts, cas []types. return nil } -func (f *Fetcher) getCABlob(ca types.CaReference) ([]byte, error) { - if blob, ok := f.client.cas[ca.Source]; ok { +func (f *Fetcher) getCABlob(ca types.Resource) ([]byte, error) { + // this is also already checked at validation time + if ca.Source == nil { + f.Logger.Crit("invalid CA: %v", ignerrors.ErrSourceRequired) + return nil, ignerrors.ErrSourceRequired + } + if blob, ok := f.client.cas[*ca.Source]; ok { return blob, nil } - u, err := url.Parse(ca.Source) + u, err := url.Parse(*ca.Source) if err != nil { f.Logger.Crit("Unable to parse CA URL: %s", err) return nil, err @@ -164,23 +170,29 @@ func (f *Fetcher) getCABlob(ca types.CaReference) ([]byte, error) { } } + var compression string + if ca.Compression != nil { + compression = *ca.Compression + } + cablob, err := f.FetchToBuffer(*u, FetchOptions{ Hash: hasher, Headers: headers, ExpectedSum: expectedSum, + Compression: compression, }) if err != nil { f.Logger.Err("Unable to fetch CA (%s): %s", u, err) return nil, err } - f.client.cas[ca.Source] = cablob + f.client.cas[*ca.Source] = cablob return cablob, nil } // RewriteCAsWithDataUrls will modify the passed in slice of CA references to // contain the actual CA file via a dataurl in their source field. -func (f *Fetcher) RewriteCAsWithDataUrls(cas []types.CaReference) error { +func (f *Fetcher) RewriteCAsWithDataUrls(cas []types.Resource) error { for i, ca := range cas { blob, err := f.getCABlob(ca) if err != nil { @@ -190,7 +202,8 @@ func (f *Fetcher) RewriteCAsWithDataUrls(cas []types.CaReference) error { // Clean HTTP headers cas[i].HTTPHeaders = nil - cas[i].Source = dataurl.EncodeBytes(blob) + encoded := dataurl.EncodeBytes(blob) + cas[i].Source = &encoded } return nil } From 3660e8cf2e111ca999aa1aa81adda1d39218f441 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Wed, 8 Apr 2020 22:21:08 -0400 Subject: [PATCH 2/2] tests: add positive tests for resource compression Also verify hashes, to ensure that Ignition's decompress-before-verify semantics are maintained (https://github.com/coreos/ignition/issues/961). --- tests/positive/files/remote.go | 43 +++++++++++++++ tests/positive/general/general.go | 87 +++++++++++++++++++++++++++++++ tests/positive/security/tls.go | 51 ++++++++++++++++++ tests/servers/servers.go | 38 ++++++++++++-- 4 files changed, 215 insertions(+), 4 deletions(-) diff --git a/tests/positive/files/remote.go b/tests/positive/files/remote.go index 605cc48eb..b06b29b11 100644 --- a/tests/positive/files/remote.go +++ b/tests/positive/files/remote.go @@ -15,12 +15,16 @@ package files import ( + "strings" + "github.com/coreos/ignition/v2/tests/register" + "github.com/coreos/ignition/v2/tests/servers" "github.com/coreos/ignition/v2/tests/types" ) func init() { register.Register(register.PositiveTest, CreateFileFromRemoteContentsHTTP()) + register.Register(register.PositiveTest, CreateFileFromRemoteContentsHTTPCompressed()) register.Register(register.PositiveTest, CreateFileFromRemoteContentsHTTPUsingHeaders()) register.Register(register.PositiveTest, CreateFileFromRemoteContentsHTTPUsingHeadersWithRedirect()) register.Register(register.PositiveTest, CreateFileFromRemoteContentsHTTPUsingOverwrittenHeaders()) @@ -62,6 +66,45 @@ func CreateFileFromRemoteContentsHTTP() types.Test { } } +func CreateFileFromRemoteContentsHTTPCompressed() types.Test { + name := "files.create.http.compressed" + in := types.GetBaseDisk() + out := types.GetBaseDisk() + config := strings.Replace(`{ + "ignition": { "version": "$version" }, + "storage": { + "files": [{ + "path": "/foo/bar", + "contents": { + "compression": "gzip", + "source": "http://127.0.0.1:8080/contents_compressed", + "verification": { + "hash": "sha512-HASH" + } + } + }] + } + }`, "HASH", servers.ContentsHash, -1) + out[0].Partitions.AddFiles("ROOT", []types.File{ + { + Node: types.Node{ + Name: "bar", + Directory: "foo", + }, + Contents: "asdf\nfdsa", + }, + }) + configMinVersion := "3.1.0-experimental" + + return types.Test{ + Name: name, + In: in, + Out: out, + Config: config, + ConfigMinVersion: configMinVersion, + } +} + func CreateFileFromRemoteContentsHTTPUsingHeaders() types.Test { name := "files.create.http.headers" in := types.GetBaseDisk() diff --git a/tests/positive/general/general.go b/tests/positive/general/general.go index b096bd54d..173c4ed7c 100644 --- a/tests/positive/general/general.go +++ b/tests/positive/general/general.go @@ -26,10 +26,12 @@ func init() { // TODO: Add S3 tests register.Register(register.PositiveTest, ReformatFilesystemAndWriteFile()) register.Register(register.PositiveTest, ReplaceConfigWithRemoteConfigHTTP()) + register.Register(register.PositiveTest, ReplaceConfigWithRemoteConfigHTTPCompressed()) register.Register(register.PositiveTest, ReplaceConfigWithRemoteConfigHTTPUsingHeaders()) register.Register(register.PositiveTest, ReplaceConfigWithRemoteConfigHTTPUsingHeadersWithRedirect()) register.Register(register.PositiveTest, ReplaceConfigWithRemoteConfigHTTPUsingOverwrittenHeaders()) register.Register(register.PositiveTest, AppendConfigWithRemoteConfigHTTP()) + register.Register(register.PositiveTest, AppendConfigWithRemoteConfigHTTPCompressed()) register.Register(register.PositiveTest, AppendConfigWithRemoteConfigHTTPUsingHeaders()) register.Register(register.PositiveTest, AppendConfigWithRemoteConfigHTTPUsingHeadersWithRedirect()) register.Register(register.PositiveTest, AppendConfigWithRemoteConfigHTTPUsingOverwrittenHeaders()) @@ -123,6 +125,42 @@ func ReplaceConfigWithRemoteConfigHTTP() types.Test { } } +func ReplaceConfigWithRemoteConfigHTTPCompressed() types.Test { + name := "config.replace.http.compressed" + in := types.GetBaseDisk() + out := types.GetBaseDisk() + config := strings.Replace(`{ + "ignition": { + "version": "$version", + "config": { + "replace": { + "compression": "gzip", + "source": "http://127.0.0.1:8080/config_compressed", + "verification": { "hash": "sha512-HASH" } + } + } + } + }`, "HASH", servers.ConfigHash, 1) + configMinVersion := "3.1.0-experimental" + out[0].Partitions.AddFiles("ROOT", []types.File{ + { + Node: types.Node{ + Name: "bar", + Directory: "foo", + }, + Contents: "example file\n", + }, + }) + + return types.Test{ + Name: name, + In: in, + Out: out, + Config: config, + ConfigMinVersion: configMinVersion, + } +} + func ReplaceConfigWithRemoteConfigHTTPUsingHeaders() types.Test { name := "config.replace.http.headers" in := types.GetBaseDisk() @@ -319,6 +357,55 @@ func AppendConfigWithRemoteConfigHTTP() types.Test { } } +func AppendConfigWithRemoteConfigHTTPCompressed() types.Test { + name := "config.merge.http.compressed" + in := types.GetBaseDisk() + out := types.GetBaseDisk() + config := strings.Replace(`{ + "ignition": { + "version": "$version", + "config": { + "merge": [{ + "compression": "gzip", + "source": "http://127.0.0.1:8080/config_compressed", + "verification": { "hash": "sha512-HASH" } + }] + } + }, + "storage": { + "files": [{ + "path": "/foo/bar2", + "contents": { "source": "data:,another%20example%20file%0A" } + }] + } + }`, "HASH", servers.ConfigHash, 1) + configMinVersion := "3.1.0-experimental" + out[0].Partitions.AddFiles("ROOT", []types.File{ + { + Node: types.Node{ + Name: "bar", + Directory: "foo", + }, + Contents: "example file\n", + }, + { + Node: types.Node{ + Name: "bar2", + Directory: "foo", + }, + Contents: "another example file\n", + }, + }) + + return types.Test{ + Name: name, + In: in, + Out: out, + Config: config, + ConfigMinVersion: configMinVersion, + } +} + func AppendConfigWithRemoteConfigHTTPUsingHeaders() types.Test { name := "config.merge.http.headers" in := types.GetBaseDisk() diff --git a/tests/positive/security/tls.go b/tests/positive/security/tls.go index 8a6884402..cd5fdb5e2 100644 --- a/tests/positive/security/tls.go +++ b/tests/positive/security/tls.go @@ -21,6 +21,7 @@ import ( "net/http/httptest" "github.com/coreos/ignition/v2/tests/register" + "github.com/coreos/ignition/v2/tests/servers" "github.com/coreos/ignition/v2/tests/types" "github.com/vincent-petithory/dataurl" @@ -38,6 +39,7 @@ func init() { register.Register(register.PositiveTest, AppendConfigCustomCert()) register.Register(register.PositiveTest, FetchFileCustomCert()) register.Register(register.PositiveTest, FetchFileCustomCertHTTP()) + register.Register(register.PositiveTest, FetchFileCustomCertHTTPCompressed()) register.Register(register.PositiveTest, FetchFileCustomCertHTTPUsingHeaders()) register.Register(register.PositiveTest, FetchFileCustomCertHTTPUsingHeadersWithRedirect()) register.Register(register.PositiveTest, FetchFileCustomCertHTTPUsingOverwrittenHeaders()) @@ -224,6 +226,55 @@ func FetchFileCustomCertHTTP() types.Test { } } +func FetchFileCustomCertHTTPCompressed() types.Test { + name := "tls.fetchfile.http.compressed" + in := types.GetBaseDisk() + out := types.GetBaseDisk() + config := fmt.Sprintf(`{ + "ignition": { + "version": "$version", + "security": { + "tls": { + "certificateAuthorities": [{ + "compression": "gzip", + "source": "http://127.0.0.1:8080/certificates_compressed", + "verification": { + "hash": "sha512-%v" + } + }] + } + } + }, + "storage": { + "files": [{ + "path": "/foo/bar", + "contents": { + "source": %q + } + }] + } + }`, servers.PublicKeyHash, customCAServer.URL) + configMinVersion := "3.1.0-experimental" + + out[0].Partitions.AddFiles("ROOT", []types.File{ + { + Node: types.Node{ + Directory: "foo", + Name: "bar", + }, + Contents: string(customCAServerFile), + }, + }) + + return types.Test{ + Name: name, + In: in, + Out: out, + Config: config, + ConfigMinVersion: configMinVersion, + } +} + func FetchFileCustomCertHTTPUsingHeaders() types.Test { name := "tls.fetchfile.http.headers" in := types.GetBaseDisk() diff --git a/tests/servers/servers.go b/tests/servers/servers.go index 74d0758d4..2aa9c4f34 100644 --- a/tests/servers/servers.go +++ b/tests/servers/servers.go @@ -16,6 +16,7 @@ package servers import ( "bytes" + "compress/gzip" "crypto/sha512" "encoding/hex" "fmt" @@ -61,10 +62,12 @@ AKbyaAqbChEy9CvDgyv6qxTYU+eeBImLKS3PH2uW5etc/69V/sDojqpH3hEffsOt -----END CERTIFICATE-----`) // export these so tests don't have to hard-code them everywhere - configRawHash = sha512.Sum512(servedConfig) - contentsRawHash = sha512.Sum512(servedContents) - ConfigHash = hex.EncodeToString(configRawHash[:]) - ContentsHash = hex.EncodeToString(contentsRawHash[:]) + configRawHash = sha512.Sum512(servedConfig) + contentsRawHash = sha512.Sum512(servedContents) + publicKeyRawHash = sha512.Sum512(servedPublicKey) + ConfigHash = hex.EncodeToString(configRawHash[:]) + ContentsHash = hex.EncodeToString(contentsRawHash[:]) + PublicKeyHash = hex.EncodeToString(publicKeyRawHash[:]) ) // HTTP Server @@ -80,6 +83,30 @@ func (server *HTTPServer) Certificates(w http.ResponseWriter, r *http.Request) { w.Write(servedPublicKey) } +func compress(contents []byte) []byte { + var buf bytes.Buffer + w := gzip.NewWriter(&buf) + if _, err := w.Write(contents); err != nil { + panic(err) + } + if err := w.Close(); err != nil { + panic(err) + } + return buf.Bytes() +} + +func (server *HTTPServer) ConfigCompressed(w http.ResponseWriter, r *http.Request) { + w.Write(compress(servedConfig)) +} + +func (server *HTTPServer) ContentsCompressed(w http.ResponseWriter, r *http.Request) { + w.Write(compress(servedContents)) +} + +func (server *HTTPServer) CertificatesCompressed(w http.ResponseWriter, r *http.Request) { + w.Write(compress(servedPublicKey)) +} + func errorHandler(w http.ResponseWriter, message string) { w.WriteHeader(http.StatusBadRequest) w.Write([]byte(message)) @@ -227,16 +254,19 @@ type HTTPServer struct{} func (server *HTTPServer) Start() { http.HandleFunc("/contents", server.Contents) + http.HandleFunc("/contents_compressed", server.ContentsCompressed) http.HandleFunc("/contents_headers", server.ContentsHeaders) http.HandleFunc("/contents_headers_redirect", server.ContentsRedirect) http.HandleFunc("/contents_headers_redirected", server.ContentsRedirected) http.HandleFunc("/contents_headers_overwrite", server.ContentsHeadersOverwrite) http.HandleFunc("/certificates", server.Certificates) + http.HandleFunc("/certificates_compressed", server.CertificatesCompressed) http.HandleFunc("/certificates_headers", server.CertificatesHeaders) http.HandleFunc("/certificates_headers_redirect", server.CertificatesRedirect) http.HandleFunc("/certificates_headers_redirected", server.CertificatesRedirected) http.HandleFunc("/certificates_headers_overwrite", server.CertificatesHeadersOverwrite) http.HandleFunc("/config", server.Config) + http.HandleFunc("/config_compressed", server.ConfigCompressed) http.HandleFunc("/config_headers", server.ConfigHeaders) http.HandleFunc("/config_headers_redirect", server.ConfigRedirect) http.HandleFunc("/config_headers_redirected", server.ConfigRedirected)