Skip to content

Fix case where it was not possible to unmarshal without strict mode #642

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.

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

Merged
merged 1 commit into from
May 23, 2025

Conversation

cpuguy83
Copy link
Member

We need to be able to unmarshal without strict mode because clients may be trying to unmarshal data from different versions of dalec.

We need to be able to unmarshal without strict mode because clients may
be trying to unmarshal data from different versions of dalec.

Signed-off-by: Brian Goff <[email protected]>
@cpuguy83 cpuguy83 requested a review from a team as a code owner May 23, 2025 02:01
@@ -101,6 +101,8 @@ type Spec struct {
Tests []*TestSpec `yaml:"tests,omitempty" json:"tests,omitempty"`

extensions extensionFields `yaml:"-" json:"-"`

decodeOpts []yaml.DecodeOption `yaml:"-" json:"-"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I am assuming this gets treated as optional or similar? not familiar with the - option

Copy link
Member Author

Choose a reason for hiding this comment

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

It tells the encoder/decoder to ignore the field.

@DannyBrito
Copy link
Contributor

We need to be able to unmarshal without strict mode because clients may be trying to unmarshal data from different versions of dalec.

I would have assumed that the client will always know the exact strict marshall as it points to a version dalec frontend image?
Will or could this introduce possible issues where we are using a no longer available or renamed field and cause unexpected behavior where something was properly handle?

@cpuguy83
Copy link
Member Author

We need to be able to unmarshal without strict mode because clients may be trying to unmarshal data from different versions of dalec.

I would have assumed that the client will always know the exact strict marshall as it points to a version dalec frontend image? Will or could this introduce possible issues where we are using a no longer available or renamed field and cause unexpected behavior where something was properly handle?

When using dalec.LoadSpec, which is what is used in dalec to unmarshal specs, it is always unmarshalling in strict mode.

Before #511 it was possible to just manually unmarshal like below and get non-strict unmarshalling, but since #511 was intorduced this is no longer possible and was an unintended side effect.

var spec dalec.Spec
yaml.Unmarshal(data, &spec)

This change is bringing that back so that clients can read fields off of specs that may have different fields.

@cpuguy83 cpuguy83 merged commit 581d584 into Azure:main May 23, 2025
38 of 40 checks passed
@cpuguy83 cpuguy83 deleted the allow_non_strict_unmarshal branch May 23, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants