dataclients/kubernetes/kubernetestest: support getting resource by name #2937
Conversation
This is useful for testing getting endpoints by service name. For #2476 Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
| pathRx: regexp.MustCompile( | ||
| "(/namespaces/([^/]+))?/(services|ingresses|routegroups|endpointslices|endpoints|secrets)", | ||
| "(?:/namespaces/([^/]+))?/(services|ingresses|routegroups|endpointslices|endpoints|secrets)(?:/(.+))?", |
There was a problem hiding this comment.
We may consider refactoring this later to use https://pkg.go.dev/net/http#hdr-Patterns added in go 1.22
There was a problem hiding this comment.
What do you think about having test like this? For you not to forget to have a look when we will update Go version.
func TestPathRx(t *testing.T) {
if runtime.Version() == "go1.22" {
t.Error("please pay attention to dataclients/kubernetes/kubernetestest/api.go:49 using http.Patterns")
}
}
There was a problem hiding this comment.
This will fail as we use 1.22 already.
We can use 1.22 features after we bump min version in go.mod
The proposed change is optional it may wait until we touch this code again.
| Items []struct { | ||
| Metadata struct { | ||
| Name string `json:"name"` |
There was a problem hiding this comment.
Should we consider namespace here as well?
There was a problem hiding this comment.
We can add it but is not used as the test api only filters by name or labels
| check(t, sec, 0, "Secret") | ||
| }) | ||
|
|
||
| t.Run("resource by name", func(t *testing.T) { |
There was a problem hiding this comment.
Should we add "resource with this name does not exist in current namespace, but exists in another"?
There was a problem hiding this comment.
Why? The result will be the same as "resource name does not exist" below
There was a problem hiding this comment.
I expected to have some probability of some bug related to it in k8s API, but maybe I am too paranoid.
|
👍 |
| get := func(t *testing.T, uri string, o interface{}) { | ||
| t.Helper() | ||
| err := getJSON(s.URL+uri, o) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
get function looks a bit weird as a name if we start to check for error. Maybe mustGet
There was a problem hiding this comment.
Its just a helper closure, does it matter?
|
👍 |
AlexanderYastrebov
deleted the
dataclients/kubernetes/kubernetestest-resource-by-name
branch
This is useful for testing getting endpoints by service name.
For #2476