Skip to content

Commit

Permalink
Revert "Add a replacement mechanism to work around nil handling in uc…
Browse files Browse the repository at this point in the history
…fg (elastic#4041)" (elastic#5044)

Since elastic/go-ucfg v0.8.4 now correctly renders empty arrays, we no longer need the nil replacement mechanism work around (elastic#4041).
Also updates the unit test TestCanonicalConfig_Render to cover that the configuration with an empty array works.
  • Loading branch information
thbkrkr authored and fantapsody committed Jan 3, 2023
1 parent 9f20ec4 commit d6a49ef
Show file tree
Hide file tree
Showing 6 changed files with 7 additions and 140 deletions.
7 changes: 1 addition & 6 deletions pkg/controller/common/settings/canonical_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,20 +155,15 @@ func (c *CanonicalConfig) HasKeys(keys []string) []string {

// Render returns the content of the configuration file,
// with fields sorted alphabetically
func (c *CanonicalConfig) Render(rs ...Replacement) ([]byte, error) {
func (c *CanonicalConfig) Render() ([]byte, error) {
if c == nil {
return []byte{}, nil
}

var out untypedDict
err := c.asUCfg().Unpack(&out)
if err != nil {
return []byte{}, err
}

for _, r := range rs {
r.apply(out)
}
return yaml.Marshal(out)
}

Expand Down
7 changes: 5 additions & 2 deletions pkg/controller/common/settings/canonical_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ import (
)

func TestCanonicalConfig_Render(t *testing.T) {
config := MustCanonicalConfig(map[string]string{
config := MustCanonicalConfig(map[string]interface{}{
"aaa": "aa a",
"bbb": "b bb",
"aab": "a a a",
"key": map[string]interface{}{"emptyarray": []string{}},
"withquotes": "aa\"bb\"aa",
"zz": "zzz z z z",
})
Expand All @@ -24,10 +25,12 @@ func TestCanonicalConfig_Render(t *testing.T) {
expected := []byte(`aaa: aa a
aab: a a a
bbb: b bb
key:
emptyarray: []
withquotes: aa"bb"aa
zz: zzz z z z
`)
require.Equal(t, expected, output)
require.Equal(t, string(expected), string(output))
}

func TestCanonicalConfig_MergeWith(t *testing.T) {
Expand Down
35 changes: 0 additions & 35 deletions pkg/controller/common/settings/replace.go

This file was deleted.

84 changes: 0 additions & 84 deletions pkg/controller/common/settings/replace_test.go

This file was deleted.

12 changes: 0 additions & 12 deletions pkg/controller/elasticsearch/settings/canonical_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,6 @@ import (
"github.com/elastic/cloud-on-k8s/pkg/controller/common/version"
)

// replaceNilNodeRoles works around a rendering issue with 0-sized string arrays/slices
// that have gone through ucfg and have been reduced to nil.
// Elasticsearch's new node.roles syntax relies on empty string arrays in YAML to express
// a coordinating only node. See https://github.com/elastic/cloud-on-k8s/issues/3718
var replaceNilNodeRoles = common.Replacement{
Path: []string{
"node", "roles",
},
Expected: nil,
Replacement: make([]string, 0),
}

// CanonicalConfig contains configuration for Elasticsearch ("elasticsearch.yml"),
// as a hierarchical key-value configuration.
type CanonicalConfig struct {
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/elasticsearch/settings/config_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func ConfigSecret(es esv1.Elasticsearch, ssetName string, configData []byte) cor

// ReconcileConfig ensures the ES config for the pod is set in the apiserver.
func ReconcileConfig(client k8s.Client, es esv1.Elasticsearch, ssetName string, config CanonicalConfig) error {
rendered, err := config.Render(replaceNilNodeRoles)
rendered, err := config.Render()
if err != nil {
return err
}
Expand Down

0 comments on commit d6a49ef

Please sign in to comment.