Skip to content

Commit 363a23e

Browse files
yasminvalimyasminvalim
committed
fcos/v1_6_exp: Add code reviews insights
1 parent 3cf4dd3 commit 363a23e

File tree

6 files changed

+101
-59
lines changed

6 files changed

+101
-59
lines changed

‎config/common/errors.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ var (
9898
ErrGeneralKernelArgumentSupport = errors.New("kernel argument customization is not supported in this spec version")
9999

100100
// Selinux Module
101-
ErrSelinuxContentNotSpecified = errors.New("field \"content\" is required")
102-
ErrSelinuxNameNotSpecified = errors.New("field \"name\" is required")
101+
ErrSelinuxContentsNotSpecified = errors.New("field \"contents\" is required")
102+
ErrSelinuxNameNotSpecified = errors.New("field \"name\" is required")
103103
)
104104

105105
type ErrUnmarshal struct {

‎config/fcos/v1_6_exp/schema.go

+23-3
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,30 @@ type GrubUser struct {
5353
}
5454

5555
type Selinux struct {
56-
Module []Module `yaml:"module"`
56+
Modules []Module `yaml:"modules"`
5757
}
5858

5959
type Module struct {
60-
Name string `yaml:"name"`
61-
Content string `yaml:"content"`
60+
Name string `yaml:"name"`
61+
Contents Resource `yaml:"contents"`
62+
}
63+
64+
type Resource struct {
65+
Compression *string `yaml:"compression"`
66+
HTTPHeaders HTTPHeaders `yaml:"http_headers"`
67+
Source *string `yaml:"source"`
68+
Inline *string `yaml:"inline"` // Added, not in ignition spec
69+
Local *string `yaml:"local"` // Added, not in ignition spec
70+
Verification Verification `yaml:"verification"`
71+
}
72+
73+
type HTTPHeader struct {
74+
Name string `yaml:"name"`
75+
Value *string `yaml:"value"`
76+
}
77+
78+
type HTTPHeaders []HTTPHeader
79+
80+
type Verification struct {
81+
Hash *string `yaml:"hash"`
6282
}

‎config/fcos/v1_6_exp/translate.go

+32-14
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package v1_6_exp
1717
import (
1818
"fmt"
1919
"strings"
20+
"text/template"
2021

2122
baseutil "github.com/coreos/butane/base/util"
2223
"github.com/coreos/butane/config/common"
@@ -55,6 +56,20 @@ const (
5556
bootV1SizeMiB = 384
5657
)
5758

59+
var (
60+
mountUnitTemplate = template.Must(template.New("unit").Parse(`
61+
# Generated by Butane
62+
[Unit]
63+
Description=Import SELinux module - {{.ModuleName}}
64+
[Service]
65+
Type=oneshot
66+
RemainAfterExit=yes
67+
ExecStart={{.CmdToExecute}}
68+
[Install]
69+
WantedBy=multi-user.target
70+
`))
71+
)
72+
5873
// Return FieldFilters for this spec.
5974
func (c Config) FieldFilters() *cutil.FieldFilters {
6075
return nil
@@ -393,14 +408,14 @@ func (c Config) handleSelinux(options common.TranslateOptions) (types.Config, tr
393408
ts := translate.NewTranslationSet("yaml", "json")
394409
var r report.Report
395410

396-
for _, module := range c.Selinux.Module {
397-
rendered = processModule(rendered, module, options, ts, r, path.New("yaml", "selinux", "module"))
411+
for i, module := range c.Selinux.Modules {
412+
rendered = processModule(rendered, module, options, ts, r, path.New("yaml", "selinux", "module", i))
398413
}
399414
return rendered, ts, r
400415
}
401416

402417
func processModule(rendered types.Config, module Module, options common.TranslateOptions, ts translate.TranslationSet, r report.Report, yamlPath path.ContextPath) types.Config {
403-
src, compression, err := baseutil.MakeDataURL([]byte(module.Content), nil, !options.NoResourceAutoCompression)
418+
src, compression, err := baseutil.MakeDataURL(([]byte(*module.Contents.Inline)), nil, !options.NoResourceAutoCompression)
404419
if err != nil {
405420
r.AddOnError(yamlPath, err)
406421
return rendered
@@ -427,18 +442,21 @@ func processModule(rendered types.Config, module Module, options common.Translat
427442
// Create systemd unit to import module
428443
cmdToExecute := "/usr/sbin/semodule -i" + modulePath
429444

445+
var contents strings.Builder
446+
err = mountUnitTemplate.Execute(&contents, map[string]interface{}{
447+
"ModuleName": module.Name,
448+
"CmdToExecute": cmdToExecute,
449+
})
450+
if err != nil {
451+
panic(err)
452+
}
453+
454+
result := contents.String()
455+
430456
rendered.Systemd.Units = append(rendered.Systemd.Units, types.Unit{
431-
Name: module.Name + ".conf",
432-
Contents: util.StrToPtr(
433-
"[Unit]\n" +
434-
"Description=Import SELinux module\n" +
435-
"[Service]\n" +
436-
"Type=oneshot\n" +
437-
"RemainAfterExit=yes\n" +
438-
"ExecStart=" + cmdToExecute + "\n" +
439-
"[Install]\n" +
440-
"WantedBy=multi-user.target\n"),
441-
Enabled: util.BoolToPtr(true),
457+
Name: module.Name + ".conf",
458+
Contents: util.StrToPtr(result),
459+
Enabled: util.BoolToPtr(true),
442460
})
443461
ts.AddFromCommonSource(yamlPath, path.New("json", "systemd"), rendered.Systemd)
444462

‎config/fcos/v1_6_exp/translate_test.go

+24-21
Original file line numberDiff line numberDiff line change
@@ -1642,20 +1642,20 @@ func TestTranslateSelinux(t *testing.T) {
16421642
cmdToExecute := "/usr/sbin/semodule -i" + "/etc/selinux/targeted/modules/active/extra/some_name.cil"
16431643
translations := []translate.Translation{
16441644
{From: path.New("yaml", "version"), To: path.New("json", "ignition", "version")},
1645-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage")},
1646-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files")},
1647-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files", 0)},
1648-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files", 0, "path")},
1649-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files", 0, "append")},
1650-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files", 0, "append", 0)},
1651-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files", 0, "append", 0, "source")},
1652-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files", 0, "append", 0, "compression")},
1653-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "systemd", "units", 0, "name")},
1654-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "systemd", "units", 0, "contents")},
1655-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "systemd", "units", 0, "enabled")},
1656-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "systemd", "units", 0)},
1657-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "systemd", "units")},
1658-
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "systemd")},
1645+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage")},
1646+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files")},
1647+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files", 0)},
1648+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files", 0, "path")},
1649+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files", 0, "append")},
1650+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files", 0, "append", 0)},
1651+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files", 0, "append", 0, "source")},
1652+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files", 0, "append", 0, "compression")},
1653+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "systemd", "units", 0, "name")},
1654+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "systemd", "units", 0, "contents")},
1655+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "systemd", "units", 0, "enabled")},
1656+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "systemd", "units", 0)},
1657+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "systemd", "units")},
1658+
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "systemd")},
16591659
}
16601660
tests := []struct {
16611661
in Config
@@ -1666,10 +1666,12 @@ func TestTranslateSelinux(t *testing.T) {
16661666
{
16671667
Config{
16681668
Selinux: Selinux{
1669-
Module: []Module{
1669+
Modules: []Module{
16701670
{
1671-
Name: "some_name",
1672-
Content: "some content here",
1671+
Name: "some_name",
1672+
Contents: Resource{
1673+
Inline: util.StrToPtr("some contents here"),
1674+
},
16731675
},
16741676
},
16751677
},
@@ -1688,7 +1690,7 @@ func TestTranslateSelinux(t *testing.T) {
16881690
FileEmbedded1: types.FileEmbedded1{
16891691
Append: []types.Resource{
16901692
{
1691-
Source: util.StrToPtr("data:,some%20content%20here"),
1693+
Source: util.StrToPtr("data:,some%20contents%20here"),
16921694
Compression: util.StrToPtr(""),
16931695
},
16941696
},
@@ -1699,11 +1701,12 @@ func TestTranslateSelinux(t *testing.T) {
16991701
Systemd: types.Systemd{
17001702
Units: []types.Unit{
17011703
{
1702-
Name: "some_name" + ".conf",
1704+
Name: "some_name.conf",
17031705
Enabled: util.BoolToPtr(true),
17041706
Contents: util.StrToPtr(
1705-
"[Unit]\n" +
1706-
"Description=Import SELinux module\n" +
1707+
"\n# Generated by Butane\n" +
1708+
"[Unit]\n" +
1709+
"Description=Import SELinux module - " + "some_name" + "\n" +
17071710
"[Service]\n" +
17081711
"Type=oneshot\n" +
17091712
"RemainAfterExit=yes\n" +

‎config/fcos/v1_6_exp/validate.go

+5-10
Original file line numberDiff line numberDiff line change
@@ -100,17 +100,12 @@ func (user GrubUser) Validate(c path.ContextPath) (r report.Report) {
100100
}
101101

102102
func (m Module) Validate(c path.ContextPath) (r report.Report) {
103-
if m.Name == "" && m.Content == "" {
104-
r.AddOnError(c.Append("name"), common.ErrSelinuxContentNotSpecified)
105-
r.AddOnError(c.Append("content"), common.ErrSelinuxContentNotSpecified)
106-
} else {
107-
if m.Name == "" {
108-
r.AddOnError(c.Append("name"), common.ErrSelinuxNameNotSpecified)
109-
}
103+
if m.Name == "" {
104+
r.AddOnError(c.Append("name"), common.ErrSelinuxNameNotSpecified)
105+
}
110106

111-
if m.Content == "" {
112-
r.AddOnError(c.Append("content"), common.ErrSelinuxContentNotSpecified)
113-
}
107+
if m.Contents.Inline == nil || *m.Contents.Inline == "" {
108+
r.AddOnError(c.Append("contents"), common.ErrSelinuxContentsNotSpecified)
114109
}
115110

116111
return r

‎config/fcos/v1_6_exp/validate_test.go

+15-9
Original file line numberDiff line numberDiff line change
@@ -489,26 +489,32 @@ func TestValidateModule(t *testing.T) {
489489
{
490490
// valid module
491491
in: Module{
492-
Content: "some content",
493-
Name: "some name",
492+
Contents: Resource{
493+
Inline: util.StrToPtr("some contents"),
494+
},
495+
Name: "some name",
494496
},
495497
out: nil,
496498
errPath: path.New("yaml"),
497499
},
498500
{
499-
// content is not specified
501+
// contents is not specified
500502
in: Module{
501-
Content: "",
502-
Name: "some name",
503+
Contents: Resource{
504+
Inline: util.StrToPtr(""),
505+
},
506+
Name: "some name",
503507
},
504-
out: common.ErrSelinuxContentNotSpecified,
505-
errPath: path.New("yaml", "content"),
508+
out: common.ErrSelinuxContentsNotSpecified,
509+
errPath: path.New("yaml", "contents"),
506510
},
507511
{
508512
// name is not specified
509513
in: Module{
510-
Name: "",
511-
Content: "some content",
514+
Name: "",
515+
Contents: Resource{
516+
Inline: util.StrToPtr("some contents"),
517+
},
512518
},
513519
out: common.ErrSelinuxNameNotSpecified,
514520
errPath: path.New("yaml", "name"),

0 commit comments

Comments
 (0)