Skip to content

fix: add context result with field #619

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

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

fix: add context result with field #619

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
18 changes: 18 additions & 0 deletions go/fn/result.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,3 +299,21 @@ func ConfigObjectResult(msg string, obj *KubeObject, severity Severity) *Result
},
}
}

func ConfigObjectResultWithField(msg string, obj *KubeObject, severity Severity, field *Field) *Result {
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 hoping this function can be smarter: user provides the field path and proposedValue, it can find out the currentValue from the path, determining the right type, and validate the proposedValue field

Also. ln304-317 can be simplified by calling ConfigObjectResult

Copy link
Contributor Author

@zyy98 zyy98 Aug 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think it is difficult to have a path and determine its currentValue, the path might contain a slice. How should I represent the slice using path? Should I add [] in the path?

Since I am hard coding the path inside my new transformer, I know in advance if this field is a slice or not. How should I convey this message to ConfigObjectResultWithField? For the cases with AdditionalLabelFields, it uses [] to indicate a slice.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest reading the KRM function specification as I mentioned in last 1:1.

Path is the JSON path of the field e.g. spec.template.spec.containers[3].resources.limits.cpu

Copy link
Contributor Author

@zyy98 zyy98 Aug 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

On second thought, this is suppose to use when logging results. All the results should be updated before the logging. There is no point getting currentValue using path @yuwenma . In implementation, the old value and new value are stored in some map, then later used in the logging.

return &Result{
Message: msg,
Severity: severity,
ResourceRef: &ResourceRef{
APIVersion: obj.GetAPIVersion(),
Kind: obj.GetKind(),
Name: obj.GetName(),
Namespace: obj.GetNamespace(),
},
Field: field,
File: &File{
Path: obj.PathAnnotation(),
Index: obj.IndexAnnotation(),
},
}
}