Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

base/v0_6_exp: Validate merged/replaced Ignition configs #476

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

base/v0_6_exp: Validate merged/replaced Ignition configs #476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion base/v0_6_exp/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,33 +17,75 @@ package v0_6_exp
import (
baseutil "github.com/coreos/butane/base/util"
"github.com/coreos/butane/config/common"

"github.com/coreos/ignition/v2/config/shared/errors"
"github.com/coreos/ignition/v2/config/util"
exp "github.com/coreos/ignition/v2/config/v3_5_experimental"
"github.com/coreos/vcontext/path"
"github.com/coreos/vcontext/report"
)

func (rs Resource) Validate(c path.ContextPath) (r report.Report) {
var field string
sources := 0
var config string
var butaneReport report.Report
if rs.Local != nil {
Adam0Brien marked this conversation as resolved.
Show resolved Hide resolved
sources++
field = "local"
config = *rs.Local
}
if rs.Inline != nil {
sources++
field = "inline"
config = *rs.Inline
}
if rs.Source != nil {
sources++
field = "source"
config = *rs.Source
}
if sources > 1 {
r.AddOnError(c.Append(field), common.ErrTooManyResourceSources)
}
if field == "local" || field == "inline" {
_, report, err := exp.Parse([]byte(config))
if len(report.Entries) > 0 {
butaneReport = ConvertToButaneReport(report, field)
r.Merge(butaneReport)
}
if err != nil {
r.AddOnError(c.Append(field), errors.ErrUnknownVersion)
}
}
return
}

func ConvertToButaneReport(ignitionReport report.Report, field string) report.Report {
var butaneRep report.Report
for _, entry := range ignitionReport.Entries {

adjustedPath := []interface{}{field}
adjustedPath = append(adjustedPath, entry.Context.Path...)


butaneEntry := report.Entry{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not sure this is everything we need.
Looping through the entries and expanding them into a new report object with those entries is good.
The issue comes with the the entry.Context path the paths are different between the ignition package and the butane package. this is mostly because we are converting from json to yaml butane is human readable so their names change.

internal to butane it looks like we already have a similar issue, we have to translate to json from yaml the direction is the inverse of what we need now though.

It has a good test to explain this here => https://github.com/coreos/butane/blob/0244d31c627890b9af64b82e2a2224ac39bbb4d4/base/v0_6_exp/translate_test.go#L573C12-L573C12

Kind: entry.Kind,
Message: entry.Message,
Context: path.ContextPath{
Path: adjustedPath, // convert ignition path to butane path
Tag: entry.Context.Tag,
},
Marker: entry.Marker,
}
butaneRep.Entries = append(butaneRep.Entries, butaneEntry)
}
return butaneRep
}

// func TranslatePath() {
// path translating logic // TODO
// }

func (fs Filesystem) Validate(c path.ContextPath) (r report.Report) {
if !util.IsTrue(fs.WithMountUnit) {
return
Expand Down
18 changes: 9 additions & 9 deletions base/v0_6_exp/validate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestValidateResource(t *testing.T) {
// inline specified
{
Resource{
Inline: util.StrToPtr("hello"),
Inline: util.StrToPtr("{\"ignition\": {\"version\": \"3.5.0\"}}"),
Compression: util.StrToPtr("gzip"),
Verification: Verification{
Hash: util.StrToPtr("this isn't validated"),
Expand All @@ -64,7 +64,7 @@ func TestValidateResource(t *testing.T) {
// local specified
{
Resource{
Local: util.StrToPtr("hello"),
Local: util.StrToPtr("{\"ignition\": {\"version\": \"3.5.0\"}}"),
Compression: util.StrToPtr("gzip"),
Verification: Verification{
Hash: util.StrToPtr("this isn't validated"),
Expand All @@ -77,7 +77,7 @@ func TestValidateResource(t *testing.T) {
{
Resource{
Source: util.StrToPtr("data:,hello"),
Inline: util.StrToPtr("hello"),
Inline: util.StrToPtr("{\"ignition\": {\"version\": \"3.5.0\"}}"),
Compression: util.StrToPtr("gzip"),
Verification: Verification{
Hash: util.StrToPtr("this isn't validated"),
Expand All @@ -90,7 +90,7 @@ func TestValidateResource(t *testing.T) {
{
Resource{
Source: util.StrToPtr("data:,hello"),
Local: util.StrToPtr("hello"),
Local: util.StrToPtr("{\"ignition\": {\"version\": \"3.5.0\"}}"),
Compression: util.StrToPtr("gzip"),
Verification: Verification{
Hash: util.StrToPtr("this isn't validated"),
Expand All @@ -99,11 +99,11 @@ func TestValidateResource(t *testing.T) {
common.ErrTooManyResourceSources,
path.New("yaml", "source"),
},
// inline + local, invalid
// // inline + local, invalid
{
Resource{
Inline: util.StrToPtr("hello"),
Local: util.StrToPtr("hello"),
Inline: util.StrToPtr("{\"ignition\": {\"version\": \"3.5.0\"}}"),
Local: util.StrToPtr("{\"ignition\": {\"version\": \"3.5.0\"}}"),
Compression: util.StrToPtr("gzip"),
Verification: Verification{
Hash: util.StrToPtr("this isn't validated"),
Expand All @@ -116,8 +116,8 @@ func TestValidateResource(t *testing.T) {
{
Resource{
Source: util.StrToPtr("data:,hello"),
Inline: util.StrToPtr("hello"),
Local: util.StrToPtr("hello"),
Inline: util.StrToPtr("{\"ignition\": {\"version\": \"3.5.0\"}}"),
Local: util.StrToPtr("{\"ignition\": {\"version\": \"3.5.0\"}}"),
Compression: util.StrToPtr("gzip"),
Verification: Verification{
Hash: util.StrToPtr("this isn't validated"),
Expand Down
7 changes: 2 additions & 5 deletions docs/release-notes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,8 @@ key](https://getfedora.org/security/).
- Warn if config attempts to reuse partition by label _(fcos 1.6.0-exp,
openshift 4.14.0)_
- Require `storage.filesystems.path` to start with `/etc` or `/var` if
`with_mount_unit` is true _(fcos 1.6.0-exp, openshift 4.14.0)_
- Stabilize OpenShift spec 4.14.0, targeting Ignition spec 3.4.0
- Add OpenShift spec 4.15.0-experimental, targeting Ignition spec
3.5.0-experimental
- Add new variant `fiot` for fedora-iot
`with_mount_unit` is true _(fcos 1.6.0-exp, openshift 4.14.0-exp)_
- Validate merged/replaced ignition configs if they are local/inline _(fcos 1.6.0-exp)_

### Bug fixes

Expand Down
187 changes: 187 additions & 0 deletions vendor/github.com/coreos/ignition/v2/config/translate/translate.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading