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.

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

Document fieldpath semantics #49

Open
Mitsuwa opened this issue Oct 12, 2023 · 2 comments
Open

Document fieldpath semantics #49

Mitsuwa opened this issue Oct 12, 2023 · 2 comments
Labels
documentation Improvements or additions to documentation

Comments

@Mitsuwa
Copy link
Contributor

Mitsuwa commented Oct 12, 2023

What happened?

Setting an annotation or label via SetValue using a label or annotation key with a "." is set incorrectly ie: prometheus.io.metrics

It will be set as

metadata:
  labels:
    prometheus:
      io:
        metrics: /metrics

It should result in

metadata:
  labels:
    prometheus.io.metrics: /metrics

This seems to be more of an issue with the SetValue in fieldpath Pave objects.

I understand that you can use SetAnnotations or SetLabels but this requires either merging the existing annotations together, which would often take longer than setting the fields individually, or requires knowing the full annotations or labels at the start. either way it would be nice to use SetValue this way as well. If it is not intended to be used this way, perhaps a comment suggesting alternatives would be good

How can we reproduce it?

using this composed_test file

package composed

import (
	"fmt"
	"testing"

	"sigs.k8s.io/yaml"
)

func TestSetValue(t *testing.T) {
	cd := New()
	cd.SetAPIVersion("example.org/v1")
	cd.SetKind("CoolComposedResource")

	cd.SetValue("metadata.labels.prometheus.io.metrics", "/metrics")
	cd.SetValue("metadata.labels.\"prometheus.io.url\"", "https://exampleprom.io")
	cd.SetValue("metadata.annotations.prometheus.io.metrics", "/metrics")
	cd.SetValue("metadata.annotations.\"prometheus.io.url\"", "https://exampleprom.io")

	y, _ := yaml.Marshal(cd)

	fmt.Println(string(y))
}

you will see the following output

$ go test -v ./...
=== RUN   TestSetValue
apiVersion: example.org/v1
kind: CoolComposedResource
metadata:
  annotations:
    prometheus:
      io:
        metrics: /metrics
        url: https://exampleprom.io
  labels:
    prometheus:
      io:
        metrics: /metrics
        url: https://exampleprom.io

--- PASS: TestSetValue (0.00s)
PASS
ok  	github.com/crossplane/function-sdk-go/resource/composed	0.226s

What environment did it happen in?

Crossplane version: Latest version

@Mitsuwa Mitsuwa added the bug Something isn't working label Oct 12, 2023
@negz
Copy link
Member

negz commented Oct 13, 2023

@Mitsuwa You can use [] syntax for this, so:

	cd.SetValue("metadata.labels[prometheus.io.metrics]", "/metrics")

The only documentation for this is hidden away in the fieldpath package - we should surface it somewhere better.

@negz negz added documentation Improvements or additions to documentation and removed bug Something isn't working labels Oct 13, 2023
@negz negz changed the title composed.SetValue() sets values incorrectly for labes or metrics containing . character Document fieldpath semantics Oct 13, 2023
@Mitsuwa
Copy link
Contributor Author

Mitsuwa commented Oct 13, 2023

oh yes! that makes sense, ty for clarifying that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants