fix: Allow serving CRs and CRDs and other improvements #125
Conversation
banjoh
force-pushed
the
em/fix-crs-and-crds
branch
from
983e9bb
to
1b2f967
Compare
* Use `unstructured` objects whenever possible especially when concrete types are unknown * Add middleware to dump requests and responses * Add getVersion API to allow running `kubectl cluster-info`. Some tools like `k9s` use this to get server info * Always allow selfsubjectaccessreviews to authorize all requests. `k9s` for example needs this * For all listing API calls, respond with empty lists not 404s * Better handling of Table responses by respecting groups Accept header value
banjoh
force-pushed
the
em/fix-crs-and-crds
branch
from
8976c1a
to
804f5c8
Compare
banjoh
changed the title
|
As a side effect of these changes, I got 
|
pkg/api/middleware.go
Outdated
| } | ||
|
|
||
| func (w *requestResponseDumper) Flush() { | ||
| w.ResponseWriter.(http.Flusher).Flush() |
There was a problem hiding this comment.
Nit for error checking
if flussher, ok := w.ResponseWriter.(http.Flusher); ok {
flusher.Flush()
}
pkg/api/middleware.go
Outdated
| return w.ResponseWriter.(http.Hijacker).Hijack() | ||
| } | ||
|
|
||
| func logObject(prefix string, o any) { |
There was a problem hiding this comment.
Why use any instead of interface{}? This breaks codebase consistency.
There was a problem hiding this comment.
I've found any a clearer type to use but I'll revert in favour of consistency
pkg/api/server.go
Outdated
| type handler struct { | ||
| clusterData sbctl.ClusterData | ||
| } | ||
| type ( |
There was a problem hiding this comment.
consistency
type <name> struct
| @@ -161,12 +173,36 @@ func (h handler) getAPI(w http.ResponseWriter, r *http.Request) { | |||
| JSON(w, http.StatusOK, apiVersions) | |||
| } | |||
|
|
|||
| func (h handler) getVersion(w http.ResponseWriter, r *http.Request) { | |||
| log.Println("called getVersion") | |||
| data, err := os.ReadFile(h.clusterData.ClusterInfoFile) | |||
There was a problem hiding this comment.
This function is tricky. While server version is returned from support bundle, this tool may not be implementing that particular version. But if this helps in some cases, it's ok. I don't really have any good suggestions.
pkg/api/server.go
Outdated
| }) | ||
|
|
||
| result = &obj | ||
| break |
There was a problem hiding this comment.
This should do nothing and continue instead.
pkg/api/server.go
Outdated
| Kind: resource, | ||
| }) | ||
|
|
||
| result = &obj |
There was a problem hiding this comment.
This should happen after the loop if result is not set to anything.
pkg/api/server.go
Outdated
| }) | ||
|
|
||
| result = &obj | ||
| break |
There was a problem hiding this comment.
Do nothing and continue. Set result to empty object if no data was found in the entire loop.
pkg/api/utils.go
Outdated
| // parseAcceptHeader parses the a header and returns a map of key value pairs | ||
| // An kubectl Accept header for example looks like this: | ||
| // application/json;as=Table;g=meta.k8s.io;v=v1beta1, application/json | ||
| func parseAcceptHeader(headers []string) map[string]string { |
There was a problem hiding this comment.
This function only works with the inputs in the test case. It will break if there are white spaces in the header for example. Is there an existing package that can be used instead? Http parameter parsing is not a new thing. It's gotta be implemented somewhere already.
There was a problem hiding this comment.
I'll search around for any existing implementation
| // Try to decode object into an unstructured object | ||
| var v unstructured.Unstructured | ||
| err = json.Unmarshal(originalData, &v) | ||
| if err == nil { |
There was a problem hiding this comment.
value of err changes a few times in this block. how about logging it every time
if err != nil {
// log and continue
}
Here are the list of changes made to achieve being able to read custom resources. I also added some improvements in the process
unstructuredobjects whenever possible especially when concrete types are unknown.--debugCLI option. They are handy when developing.kubectl cluster-info. Some tools likek9suse this to get server infoselfsubjectaccessreviewstostatus.allowed=trueso as to authorise all requests.k9sfor example needs thiskubectlbehave as expected whenever there are no resources found[evans] $ kubectl get pods -n not-there No resources found in not-there namespace.Acceptheader valuesFixes: #48
Partially addresses #75