Skip to content

Commit

Permalink
fcos/v1_6_exp: Add code reviews insights
Browse files Browse the repository at this point in the history
  • Loading branch information
@yasminvalim
yasminvalim committed Feb 19, 2024
1 parent 3cf4dd3 commit 363a23e
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 59 deletions.
4 changes: 2 additions & 2 deletions config/common/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ var (
ErrGeneralKernelArgumentSupport = errors.New("kernel argument customization is not supported in this spec version")

// Selinux Module
ErrSelinuxContentNotSpecified = errors.New("field \"content\" is required")
ErrSelinuxNameNotSpecified = errors.New("field \"name\" is required")
ErrSelinuxContentsNotSpecified = errors.New("field \"contents\" is required")
ErrSelinuxNameNotSpecified = errors.New("field \"name\" is required")
)

type ErrUnmarshal struct {
Expand Down
26 changes: 23 additions & 3 deletions config/fcos/v1_6_exp/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,30 @@ type GrubUser struct {
}

type Selinux struct {
Module []Module `yaml:"module"`
Modules []Module `yaml:"modules"`
}

type Module struct {
Name string `yaml:"name"`
Content string `yaml:"content"`
Name string `yaml:"name"`
Contents Resource `yaml:"contents"`
}

type Resource struct {
Compression *string `yaml:"compression"`
HTTPHeaders HTTPHeaders `yaml:"http_headers"`
Source *string `yaml:"source"`
Inline *string `yaml:"inline"` // Added, not in ignition spec
Local *string `yaml:"local"` // Added, not in ignition spec
Verification Verification `yaml:"verification"`
}

type HTTPHeader struct {
Name string `yaml:"name"`
Value *string `yaml:"value"`
}

type HTTPHeaders []HTTPHeader

type Verification struct {
Hash *string `yaml:"hash"`
}
46 changes: 32 additions & 14 deletions config/fcos/v1_6_exp/translate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package v1_6_exp
import (
"fmt"
"strings"
"text/template"

baseutil "github.com/coreos/butane/base/util"
"github.com/coreos/butane/config/common"
Expand Down Expand Up @@ -55,6 +56,20 @@ const (
bootV1SizeMiB = 384
)

var (
mountUnitTemplate = template.Must(template.New("unit").Parse(`
# Generated by Butane
[Unit]
Description=Import SELinux module - {{.ModuleName}}
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart={{.CmdToExecute}}
[Install]
WantedBy=multi-user.target
`))
)

// Return FieldFilters for this spec.
func (c Config) FieldFilters() *cutil.FieldFilters {
return nil
Expand Down Expand Up @@ -393,14 +408,14 @@ func (c Config) handleSelinux(options common.TranslateOptions) (types.Config, tr
ts := translate.NewTranslationSet("yaml", "json")
var r report.Report

for _, module := range c.Selinux.Module {
rendered = processModule(rendered, module, options, ts, r, path.New("yaml", "selinux", "module"))
for i, module := range c.Selinux.Modules {
rendered = processModule(rendered, module, options, ts, r, path.New("yaml", "selinux", "module", i))
}
return rendered, ts, r
}

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

var contents strings.Builder
err = mountUnitTemplate.Execute(&contents, map[string]interface{}{
"ModuleName": module.Name,
"CmdToExecute": cmdToExecute,
})
if err != nil {
panic(err)
}

result := contents.String()

rendered.Systemd.Units = append(rendered.Systemd.Units, types.Unit{
Name: module.Name + ".conf",
Contents: util.StrToPtr(
"[Unit]\n" +
"Description=Import SELinux module\n" +
"[Service]\n" +
"Type=oneshot\n" +
"RemainAfterExit=yes\n" +
"ExecStart=" + cmdToExecute + "\n" +
"[Install]\n" +
"WantedBy=multi-user.target\n"),
Enabled: util.BoolToPtr(true),
Name: module.Name + ".conf",
Contents: util.StrToPtr(result),
Enabled: util.BoolToPtr(true),
})
ts.AddFromCommonSource(yamlPath, path.New("json", "systemd"), rendered.Systemd)

Expand Down
45 changes: 24 additions & 21 deletions config/fcos/v1_6_exp/translate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1642,20 +1642,20 @@ func TestTranslateSelinux(t *testing.T) {
cmdToExecute := "/usr/sbin/semodule -i" + "/etc/selinux/targeted/modules/active/extra/some_name.cil"
translations := []translate.Translation{
{From: path.New("yaml", "version"), To: path.New("json", "ignition", "version")},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage")},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files")},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files", 0)},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files", 0, "path")},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files", 0, "append")},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files", 0, "append", 0)},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files", 0, "append", 0, "source")},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "storage", "files", 0, "append", 0, "compression")},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "systemd", "units", 0, "name")},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "systemd", "units", 0, "contents")},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "systemd", "units", 0, "enabled")},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "systemd", "units", 0)},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "systemd", "units")},
{From: path.New("yaml", "selinux", "module"), To: path.New("json", "systemd")},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage")},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files")},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files", 0)},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files", 0, "path")},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files", 0, "append")},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files", 0, "append", 0)},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files", 0, "append", 0, "source")},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "storage", "files", 0, "append", 0, "compression")},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "systemd", "units", 0, "name")},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "systemd", "units", 0, "contents")},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "systemd", "units", 0, "enabled")},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "systemd", "units", 0)},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "systemd", "units")},
{From: path.New("yaml", "selinux", "module", 0), To: path.New("json", "systemd")},
}
tests := []struct {
in Config
Expand All @@ -1666,10 +1666,12 @@ func TestTranslateSelinux(t *testing.T) {
{
Config{
Selinux: Selinux{
Module: []Module{
Modules: []Module{
{
Name: "some_name",
Content: "some content here",
Name: "some_name",
Contents: Resource{
Inline: util.StrToPtr("some contents here"),
},
},
},
},
Expand All @@ -1688,7 +1690,7 @@ func TestTranslateSelinux(t *testing.T) {
FileEmbedded1: types.FileEmbedded1{
Append: []types.Resource{
{
Source: util.StrToPtr("data:,some%20content%20here"),
Source: util.StrToPtr("data:,some%20contents%20here"),
Compression: util.StrToPtr(""),
},
},
Expand All @@ -1699,11 +1701,12 @@ func TestTranslateSelinux(t *testing.T) {
Systemd: types.Systemd{
Units: []types.Unit{
{
Name: "some_name" + ".conf",
Name: "some_name.conf",
Enabled: util.BoolToPtr(true),
Contents: util.StrToPtr(
"[Unit]\n" +
"Description=Import SELinux module\n" +
"\n# Generated by Butane\n" +
"[Unit]\n" +
"Description=Import SELinux module - " + "some_name" + "\n" +
"[Service]\n" +
"Type=oneshot\n" +
"RemainAfterExit=yes\n" +
Expand Down
15 changes: 5 additions & 10 deletions config/fcos/v1_6_exp/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,17 +100,12 @@ func (user GrubUser) Validate(c path.ContextPath) (r report.Report) {
}

func (m Module) Validate(c path.ContextPath) (r report.Report) {
if m.Name == "" && m.Content == "" {
r.AddOnError(c.Append("name"), common.ErrSelinuxContentNotSpecified)
r.AddOnError(c.Append("content"), common.ErrSelinuxContentNotSpecified)
} else {
if m.Name == "" {
r.AddOnError(c.Append("name"), common.ErrSelinuxNameNotSpecified)
}
if m.Name == "" {
r.AddOnError(c.Append("name"), common.ErrSelinuxNameNotSpecified)
}

if m.Content == "" {
r.AddOnError(c.Append("content"), common.ErrSelinuxContentNotSpecified)
}
if m.Contents.Inline == nil || *m.Contents.Inline == "" {
r.AddOnError(c.Append("contents"), common.ErrSelinuxContentsNotSpecified)
}

return r
Expand Down
24 changes: 15 additions & 9 deletions config/fcos/v1_6_exp/validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,26 +489,32 @@ func TestValidateModule(t *testing.T) {
{
// valid module
in: Module{
Content: "some content",
Name: "some name",
Contents: Resource{
Inline: util.StrToPtr("some contents"),
},
Name: "some name",
},
out: nil,
errPath: path.New("yaml"),
},
{
// content is not specified
// contents is not specified
in: Module{
Content: "",
Name: "some name",
Contents: Resource{
Inline: util.StrToPtr(""),
},
Name: "some name",
},
out: common.ErrSelinuxContentNotSpecified,
errPath: path.New("yaml", "content"),
out: common.ErrSelinuxContentsNotSpecified,
errPath: path.New("yaml", "contents"),
},
{
// name is not specified
in: Module{
Name: "",
Content: "some content",
Name: "",
Contents: Resource{
Inline: util.StrToPtr("some contents"),
},
},
out: common.ErrSelinuxNameNotSpecified,
errPath: path.New("yaml", "name"),
Expand Down

0 comments on commit 363a23e

Please sign in to comment.