Skip to content

Commit

Permalink
Merge pull request #244 from bgilbert/compression
Browse files Browse the repository at this point in the history 
config/openshift/*: disable automatic file compression; fail if compression manually configured
  • Loading branch information
@bgilbert
bgilbert authored Jun 10, 2021
2 parents 6ca3775 + 023fdd4 commit e235610
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 11 deletions.
23 changes: 12 additions & 11 deletions config/common/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,16 @@ var (
ErrTooFewMirrorDevices = errors.New("mirroring requires at least two devices")

// MachineConfigs
ErrFieldElided = errors.New("field ignored in raw mode")
ErrNameRequired = errors.New("metadata.name is required")
ErrRoleRequired = errors.New("machineconfiguration.openshift.io/role label is required")
ErrInvalidKernelType = errors.New("must be empty, \"default\", or \"realtime\"")
ErrBtrfsSupport = errors.New("btrfs is not supported in this spec version")
ErrDirectorySupport = errors.New("directories are not supported in this spec version")
ErrFileAppendSupport = errors.New("appending to files is not supported in this spec version")
ErrLinkSupport = errors.New("links are not supported in this spec version")
ErrGroupSupport = errors.New("groups are not supported in this spec version")
ErrUserFieldSupport = errors.New("fields other than \"name\" and \"ssh_authorized_keys\" are not supported in this spec version")
ErrUserNameSupport = errors.New("users other than \"core\" are not supported in this spec version")
ErrFieldElided = errors.New("field ignored in raw mode")
ErrNameRequired = errors.New("metadata.name is required")
ErrRoleRequired = errors.New("machineconfiguration.openshift.io/role label is required")
ErrInvalidKernelType = errors.New("must be empty, \"default\", or \"realtime\"")
ErrBtrfsSupport = errors.New("btrfs is not supported in this spec version")
ErrDirectorySupport = errors.New("directories are not supported in this spec version")
ErrFileAppendSupport = errors.New("appending to files is not supported in this spec version")
ErrFileCompressionSupport = errors.New("file compression is not supported in this spec version")
ErrLinkSupport = errors.New("links are not supported in this spec version")
ErrGroupSupport = errors.New("groups are not supported in this spec version")
ErrUserFieldSupport = errors.New("fields other than \"name\" and \"ssh_authorized_keys\" are not supported in this spec version")
ErrUserNameSupport = errors.New("users other than \"core\" are not supported in this spec version")
)
13 changes: 13 additions & 0 deletions config/openshift/v4_8/translate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ const (
// can be tracked back to their source in the source config. No config
// validation is performed on input or output.
func (c Config) ToMachineConfig4_8Unvalidated(options common.TranslateOptions) (result.MachineConfig, translate.TranslationSet, report.Report) {
// disable inline resource compression since the MCO doesn't support it
// https://bugzilla.redhat.com/show_bug.cgi?id=1970218
options.NoResourceAutoCompression = true

cfg, ts, r := c.Config.ToIgn3_2Unvalidated(options)
if r.IsFatal() {
return result.MachineConfig{}, ts, r
Expand Down Expand Up @@ -212,6 +216,10 @@ func validateMCOSupport(mc result.MachineConfig, ts translate.TranslationSet) re
// is provisioned, and the struct contains unsupported fields, MCD
// will mark the node degraded, even if the change only affects
// supported fields. We reject these.
//
// BUGGED - Ignored by the MCD but not by Ignition. Ignition
// correctly applies the setting, but the MCD doesn't, and writes
// incorrect state to the node.

var r report.Report
for i := range mc.Spec.Config.Storage.Directories {
Expand All @@ -223,6 +231,11 @@ func validateMCOSupport(mc result.MachineConfig, ts translate.TranslationSet) re
// FORBIDDEN
r.AddOnError(path.New("json", "spec", "config", "storage", "files", i, "append"), common.ErrFileAppendSupport)
}
if util.NotEmpty(file.Contents.Compression) {
// BUGGED
// https://bugzilla.redhat.com/show_bug.cgi?id=1970218
r.AddOnError(path.New("json", "spec", "config", "storage", "files", i, "contents", "compression"), common.ErrFileCompressionSupport)
}
}
for i := range mc.Spec.Config.Storage.Links {
// IMMUTABLE
Expand Down
76 changes: 76 additions & 0 deletions config/openshift/v4_8/translate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func TestElidedFieldWarning(t *testing.T) {
}

func TestTranslateConfig(t *testing.T) {
zzz := "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz"
tests := []struct {
in Config
out result.MachineConfig
Expand Down Expand Up @@ -96,6 +97,76 @@ func TestTranslateConfig(t *testing.T) {
{path.New("yaml", "version"), path.New("json", "spec", "config", "ignition", "version")},
},
},
// ensure automatic compression is disabled
{
Config{
Metadata: Metadata{
Name: "z",
Labels: map[string]string{
ROLE_LABEL_KEY: "z",
},
},
Config: fcos.Config{
Config: base.Config{
Storage: base.Storage{
Files: []base.File{
{
Path: "/z",
Contents: base.Resource{
Inline: util.StrToPtr(zzz),
},
},
},
},
},
},
},
result.MachineConfig{
ApiVersion: result.MC_API_VERSION,
Kind: result.MC_KIND,
Metadata: result.Metadata{
Name: "z",
Labels: map[string]string{
ROLE_LABEL_KEY: "z",
},
},
Spec: result.Spec{
Config: types.Config{
Ignition: types.Ignition{
Version: "3.2.0",
},
Storage: types.Storage{
Files: []types.File{
{
Node: types.Node{
Path: "/z",
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:," + zzz),
},
},
},
},
},
},
},
},
[]translate.Translation{
{path.New("yaml", "version"), path.New("json", "apiVersion")},
{path.New("yaml", "version"), path.New("json", "kind")},
{path.New("yaml", "version"), path.New("json", "spec")},
{path.New("yaml"), path.New("json", "spec", "config")},
{path.New("yaml", "ignition"), path.New("json", "spec", "config", "ignition")},
{path.New("yaml", "version"), path.New("json", "spec", "config", "ignition", "version")},
{path.New("yaml", "storage"), path.New("json", "spec", "config", "storage")},
{path.New("yaml", "storage", "files"), path.New("json", "spec", "config", "storage", "files")},
{path.New("yaml", "storage", "files", 0), path.New("json", "spec", "config", "storage", "files", 0)},
{path.New("yaml", "storage", "files", 0, "path"), path.New("json", "spec", "config", "storage", "files", 0, "path")},
{path.New("yaml", "storage", "files", 0, "contents"), path.New("json", "spec", "config", "storage", "files", 0, "contents")},
{path.New("yaml", "storage", "files", 0, "contents", "inline"), path.New("json", "spec", "config", "storage", "files", 0, "contents", "source")},
},
},
// FIPS
{
Config{
Expand Down Expand Up @@ -351,6 +422,10 @@ func TestValidateSupport(t *testing.T) {
Inline: util.StrToPtr("z"),
},
},
Contents: base.Resource{
Inline: util.StrToPtr("z"),
Compression: util.StrToPtr("gzip"),
},
},
},
Filesystems: []base.Filesystem{
Expand Down Expand Up @@ -408,6 +483,7 @@ func TestValidateSupport(t *testing.T) {
{report.Error, common.ErrBtrfsSupport, path.New("yaml", "storage", "filesystems", 0, "format")},
{report.Error, common.ErrDirectorySupport, path.New("yaml", "storage", "directories", 0)},
{report.Error, common.ErrFileAppendSupport, path.New("yaml", "storage", "files", 1, "append")},
{report.Error, common.ErrFileCompressionSupport, path.New("yaml", "storage", "files", 1, "contents", "compression")},
{report.Error, common.ErrLinkSupport, path.New("yaml", "storage", "links", 0)},
{report.Error, common.ErrGroupSupport, path.New("yaml", "passwd", "groups", 0)},
{report.Error, common.ErrUserFieldSupport, path.New("yaml", "passwd", "users", 0, "gecos")},
Expand Down
13 changes: 13 additions & 0 deletions config/openshift/v4_9_exp/translate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ const (
// can be tracked back to their source in the source config. No config
// validation is performed on input or output.
func (c Config) ToMachineConfig4_9Unvalidated(options common.TranslateOptions) (result.MachineConfig, translate.TranslationSet, report.Report) {
// disable inline resource compression since the MCO doesn't support it
// https://bugzilla.redhat.com/show_bug.cgi?id=1970218
options.NoResourceAutoCompression = true

cfg, ts, r := c.Config.ToIgn3_3Unvalidated(options)
if r.IsFatal() {
return result.MachineConfig{}, ts, r
Expand Down Expand Up @@ -212,6 +216,10 @@ func validateMCOSupport(mc result.MachineConfig, ts translate.TranslationSet) re
// is provisioned, and the struct contains unsupported fields, MCD
// will mark the node degraded, even if the change only affects
// supported fields. We reject these.
//
// BUGGED - Ignored by the MCD but not by Ignition. Ignition
// correctly applies the setting, but the MCD doesn't, and writes
// incorrect state to the node.

var r report.Report
for i := range mc.Spec.Config.Storage.Directories {
Expand All @@ -223,6 +231,11 @@ func validateMCOSupport(mc result.MachineConfig, ts translate.TranslationSet) re
// FORBIDDEN
r.AddOnError(path.New("json", "spec", "config", "storage", "files", i, "append"), common.ErrFileAppendSupport)
}
if util.NotEmpty(file.Contents.Compression) {
// BUGGED
// https://bugzilla.redhat.com/show_bug.cgi?id=1970218
r.AddOnError(path.New("json", "spec", "config", "storage", "files", i, "contents", "compression"), common.ErrFileCompressionSupport)
}
}
for i := range mc.Spec.Config.Storage.Links {
// IMMUTABLE
Expand Down
76 changes: 76 additions & 0 deletions config/openshift/v4_9_exp/translate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func TestElidedFieldWarning(t *testing.T) {
}

func TestTranslateConfig(t *testing.T) {
zzz := "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz"
tests := []struct {
in Config
out result.MachineConfig
Expand Down Expand Up @@ -96,6 +97,76 @@ func TestTranslateConfig(t *testing.T) {
{path.New("yaml", "version"), path.New("json", "spec", "config", "ignition", "version")},
},
},
// ensure automatic compression is disabled
{
Config{
Metadata: Metadata{
Name: "z",
Labels: map[string]string{
ROLE_LABEL_KEY: "z",
},
},
Config: fcos.Config{
Config: base.Config{
Storage: base.Storage{
Files: []base.File{
{
Path: "/z",
Contents: base.Resource{
Inline: util.StrToPtr(zzz),
},
},
},
},
},
},
},
result.MachineConfig{
ApiVersion: result.MC_API_VERSION,
Kind: result.MC_KIND,
Metadata: result.Metadata{
Name: "z",
Labels: map[string]string{
ROLE_LABEL_KEY: "z",
},
},
Spec: result.Spec{
Config: types.Config{
Ignition: types.Ignition{
Version: "3.3.0-experimental",
},
Storage: types.Storage{
Files: []types.File{
{
Node: types.Node{
Path: "/z",
},
FileEmbedded1: types.FileEmbedded1{
Contents: types.Resource{
Source: util.StrToPtr("data:," + zzz),
},
},
},
},
},
},
},
},
[]translate.Translation{
{path.New("yaml", "version"), path.New("json", "apiVersion")},
{path.New("yaml", "version"), path.New("json", "kind")},
{path.New("yaml", "version"), path.New("json", "spec")},
{path.New("yaml"), path.New("json", "spec", "config")},
{path.New("yaml", "ignition"), path.New("json", "spec", "config", "ignition")},
{path.New("yaml", "version"), path.New("json", "spec", "config", "ignition", "version")},
{path.New("yaml", "storage"), path.New("json", "spec", "config", "storage")},
{path.New("yaml", "storage", "files"), path.New("json", "spec", "config", "storage", "files")},
{path.New("yaml", "storage", "files", 0), path.New("json", "spec", "config", "storage", "files", 0)},
{path.New("yaml", "storage", "files", 0, "path"), path.New("json", "spec", "config", "storage", "files", 0, "path")},
{path.New("yaml", "storage", "files", 0, "contents"), path.New("json", "spec", "config", "storage", "files", 0, "contents")},
{path.New("yaml", "storage", "files", 0, "contents", "inline"), path.New("json", "spec", "config", "storage", "files", 0, "contents", "source")},
},
},
// FIPS
{
Config{
Expand Down Expand Up @@ -351,6 +422,10 @@ func TestValidateSupport(t *testing.T) {
Inline: util.StrToPtr("z"),
},
},
Contents: base.Resource{
Inline: util.StrToPtr("z"),
Compression: util.StrToPtr("gzip"),
},
},
},
Filesystems: []base.Filesystem{
Expand Down Expand Up @@ -408,6 +483,7 @@ func TestValidateSupport(t *testing.T) {
{report.Error, common.ErrBtrfsSupport, path.New("yaml", "storage", "filesystems", 0, "format")},
{report.Error, common.ErrDirectorySupport, path.New("yaml", "storage", "directories", 0)},
{report.Error, common.ErrFileAppendSupport, path.New("yaml", "storage", "files", 1, "append")},
{report.Error, common.ErrFileCompressionSupport, path.New("yaml", "storage", "files", 1, "contents", "compression")},
{report.Error, common.ErrLinkSupport, path.New("yaml", "storage", "links", 0)},
{report.Error, common.ErrGroupSupport, path.New("yaml", "passwd", "groups", 0)},
{report.Error, common.ErrUserFieldSupport, path.New("yaml", "passwd", "users", 0, "gecos")},
Expand Down

0 comments on commit e235610

Please sign in to comment.