Skip to content

Commit

Permalink
Add custom MarshalYAML() method to component to fix YAML marshaling o…
Browse files Browse the repository at this point in the history
…f error values (#3835)

* add custom MarshalYAML to component

* add changelog

* add nil check

* update tests

---------

Co-authored-by: Pierre HILBERT <[email protected]>
  • Loading branch information
fearful-symmetry and pierrehilbert authored Nov 30, 2023
1 parent 4a8661d commit 93f159a
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Kind can be one of:
# - breaking-change: a change to previously-documented behavior
# - deprecation: functionality that is being removed in a later release
# - bug-fix: fixes a problem in a previous version
# - enhancement: extends functionality but does not break or fix existing behavior
# - feature: new functionality
# - known-issue: problems that we are aware of in a given version
# - security: impacts on the security of a product or a user’s deployment.
# - upgrade: important information for someone upgrading from a prior version
# - other: does not fit into any of the other categories
kind: bug-fix

# Change summary; a 80ish characters long description of the change.
summary: custom-yaml-marshal-for-component

# Long description; in case the summary is not enough to describe the change
# this field accommodate a description without length limits.
# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
description: Create a custom MarshalYAML() method to properly handle error fields

# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
component: component

# PR URL; optional; the PR number that added the changeset.
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
# Please provide it if you are adding a fragment for a different PR.
pr: https://github.com/elastic/elastic-agent/pull/3835

# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
# If not present is automatically filled by the tooling with the issue linked to the PR number.
issue: https://github.com/elastic/elastic-agent/issues/2940
Original file line number Diff line number Diff line change
Expand Up @@ -383,12 +383,10 @@ func TestDiagnosticComponentsActual(t *testing.T) {
},
}

// The error values here shouldn't really be empty, this is a known bug, see
// https://github.com/elastic/elastic-agent/issues/2940
expected := `
components:
- id: component-1
error: {}
error: "component error"
input_type: "test-input"
output_type: "test-output"
units:
Expand Down
13 changes: 12 additions & 1 deletion pkg/component/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,11 @@ type Component struct {

// Err used when there is an error with running this input. Used by the runtime to alert
// the reason that all of these units are failed.
Err error `yaml:"error,omitempty"`
Err error `yaml:"-"`
// the YAML marshaller won't handle `error` values, since they don't implement MarshalYAML()
// the Component's own MarshalYAML method needs to handle this, and place any error values here instead of `Err`,
// so they can properly be rendered as a string
ErrMsg string `yaml:"error,omitempty"`

// InputSpec on how the input should run. (not set when ShipperSpec set)
InputSpec *InputRuntimeSpec `yaml:"input_spec,omitempty"`
Expand Down Expand Up @@ -187,6 +191,13 @@ type Component struct {
ShipperRef *ShipperReference `yaml:"shipper,omitempty"`
}

func (c Component) MarshalYAML() (interface{}, error) {
if c.Err != nil {
c.ErrMsg = c.Err.Error()
}
return c, nil
}

// Type returns the type of the component.
func (c *Component) Type() string {
if c.InputSpec != nil {
Expand Down
25 changes: 25 additions & 0 deletions pkg/component/component_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,31 @@ import (
"github.com/stretchr/testify/require"
)

// fake error type used for the test below
type testErr struct {
data string
}

func (t testErr) Error() string {
return t.data
}

func TestComponentMarshalError(t *testing.T) {
testComponent := Component{
ID: "test-device",
Err: testErr{data: "test error value"},
}
componentConfigs := []Component{testComponent}

outData, err := yaml.Marshal(struct {
Components []Component `yaml:"components"`
}{
Components: componentConfigs,
})
require.NoError(t, err)
require.Contains(t, string(outData), "test error value")
}

func TestToComponents(t *testing.T) {
linuxAMD64Platform := PlatformDetail{
Platform: Platform{
Expand Down

0 comments on commit 93f159a

Please sign in to comment.