diff --git a/pkg/manifestclient/discovery_reader.go b/pkg/manifestclient/discovery_reader.go new file mode 100644 index 0000000000..e5126561cb --- /dev/null +++ b/pkg/manifestclient/discovery_reader.go @@ -0,0 +1,150 @@ +package manifestclient + +import ( + "embed" + "errors" + "fmt" + "io/fs" + "path/filepath" + "sync" + + apidiscoveryv2 "k8s.io/api/apidiscovery/v2" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/json" + apirequest "k8s.io/apiserver/pkg/endpoints/request" + "sigs.k8s.io/yaml" +) + +type kindData struct { + kind schema.GroupVersionKind + listKind schema.GroupVersionKind + err error +} + +func newDiscoveryReader(content fs.FS) *discoveryReader { + return &discoveryReader{ + sourceFS: content, + kindForResource: make(map[schema.GroupVersionResource]kindData), + } +} + +type discoveryReader struct { + kindForResource map[schema.GroupVersionResource]kindData + + sourceFS fs.FS + lock sync.RWMutex +} + +func (dr *discoveryReader) getKindForResource(gvr schema.GroupVersionResource) (kindData, error) { + dr.lock.RLock() + kindForGVR, ok := dr.kindForResource[gvr] + if ok { + defer dr.lock.RUnlock() + return kindForGVR, kindForGVR.err + } + dr.lock.RUnlock() + + dr.lock.Lock() + defer dr.lock.Unlock() + + kindForGVR, ok = dr.kindForResource[gvr] + if ok { + return kindForGVR, kindForGVR.err + } + + discoveryPath := "/apis" + if len(gvr.Group) == 0 { + discoveryPath = "/api" + } + discoveryBytes, err := dr.getGroupResourceDiscovery(&apirequest.RequestInfo{Path: discoveryPath}) + if err != nil { + kindForGVR.err = fmt.Errorf("error reading discovery: %w", err) + dr.kindForResource[gvr] = kindForGVR + return kindForGVR, kindForGVR.err + } + + discoveryInfo := &apidiscoveryv2.APIGroupDiscoveryList{} + if err := json.Unmarshal(discoveryBytes, discoveryInfo); err != nil { + kindForGVR.err = fmt.Errorf("error unmarshalling discovery: %w", err) + dr.kindForResource[gvr] = kindForGVR + return kindForGVR, kindForGVR.err + } + + kindForGVR.err = fmt.Errorf("did not find kind for %v\n", gvr) + for _, groupInfo := range discoveryInfo.Items { + if groupInfo.Name != gvr.Group { + continue + } + for _, versionInfo := range groupInfo.Versions { + if versionInfo.Version != gvr.Version { + continue + } + for _, resourceInfo := range versionInfo.Resources { + if resourceInfo.Resource != gvr.Resource { + continue + } + if resourceInfo.ResponseKind == nil { + continue + } + kindForGVR.kind = schema.GroupVersionKind{ + Group: gvr.Group, + Version: gvr.Version, + Kind: resourceInfo.ResponseKind.Kind, + } + if len(resourceInfo.ResponseKind.Group) > 0 { + kindForGVR.kind.Group = resourceInfo.ResponseKind.Group + } + if len(resourceInfo.ResponseKind.Version) > 0 { + kindForGVR.kind.Version = resourceInfo.ResponseKind.Version + } + kindForGVR.listKind = schema.GroupVersionKind{ + Group: kindForGVR.kind.Group, + Version: kindForGVR.kind.Version, + Kind: resourceInfo.ResponseKind.Kind + "List", + } + kindForGVR.err = nil + dr.kindForResource[gvr] = kindForGVR + return kindForGVR, kindForGVR.err + } + } + } + + dr.kindForResource[gvr] = kindForGVR + return kindForGVR, kindForGVR.err +} + +func (dr *discoveryReader) getGroupResourceDiscovery(requestInfo *apirequest.RequestInfo) ([]byte, error) { + switch { + case requestInfo.Path == "/api": + return dr.getAggregatedDiscoveryForURL("aggregated-discovery-api.yaml", requestInfo.Path) + case requestInfo.Path == "/apis": + return dr.getAggregatedDiscoveryForURL("aggregated-discovery-apis.yaml", requestInfo.Path) + default: + // TODO can probably do better + return nil, fmt.Errorf("unsupported discovery path: %q", requestInfo.Path) + } +} + +func (dr *discoveryReader) getAggregatedDiscoveryForURL(filename, url string) ([]byte, error) { + discoveryBytes, err := fs.ReadFile(dr.sourceFS, filename) + if errors.Is(err, fs.ErrNotExist) { + discoveryBytes, err = fs.ReadFile(defaultDiscovery, filepath.Join("default-discovery", filename)) + } + if err != nil { + return nil, fmt.Errorf("error reading discovery: %w", err) + } + + apiMap := map[string]interface{}{} + if err := yaml.Unmarshal(discoveryBytes, &apiMap); err != nil { + return nil, fmt.Errorf("discovery %q unmarshal failed: %w", url, err) + } + apiJSON, err := json.Marshal(apiMap) + if err != nil { + return nil, fmt.Errorf("discovery %q marshal failed: %w", url, err) + } + + return apiJSON, err +} + +//go:embed default-discovery +var defaultDiscovery embed.FS diff --git a/pkg/manifestclient/group_resource_discovery.go b/pkg/manifestclient/group_resource_discovery.go deleted file mode 100644 index 8d38883576..0000000000 --- a/pkg/manifestclient/group_resource_discovery.go +++ /dev/null @@ -1,46 +0,0 @@ -package manifestclient - -import ( - "errors" - "fmt" - "io/fs" - "path/filepath" - - "k8s.io/apimachinery/pkg/util/json" - "sigs.k8s.io/yaml" - - apirequest "k8s.io/apiserver/pkg/endpoints/request" -) - -func (mrt *manifestRoundTripper) getGroupResourceDiscovery(requestInfo *apirequest.RequestInfo) ([]byte, error) { - switch { - case requestInfo.Path == "/api": - return mrt.getAggregatedDiscoveryForURL("aggregated-discovery-api.yaml", requestInfo.Path) - case requestInfo.Path == "/apis": - return mrt.getAggregatedDiscoveryForURL("aggregated-discovery-apis.yaml", requestInfo.Path) - default: - // TODO can probably do better - return nil, fmt.Errorf("unsupported discovery path: %q", requestInfo.Path) - } -} - -func (mrt *manifestRoundTripper) getAggregatedDiscoveryForURL(filename, url string) ([]byte, error) { - discoveryBytes, err := fs.ReadFile(mrt.sourceFS, filename) - if errors.Is(err, fs.ErrNotExist) { - discoveryBytes, err = fs.ReadFile(defaultDiscovery, filepath.Join("default-discovery", filename)) - } - if err != nil { - return nil, fmt.Errorf("error reading discovery: %w", err) - } - - apiMap := map[string]interface{}{} - if err := yaml.Unmarshal(discoveryBytes, &apiMap); err != nil { - return nil, fmt.Errorf("discovery %q unmarshal failed: %w", url, err) - } - apiJSON, err := json.Marshal(apiMap) - if err != nil { - return nil, fmt.Errorf("discovery %q marshal failed: %w", url, err) - } - - return apiJSON, err -} diff --git a/pkg/manifestclient/list.go b/pkg/manifestclient/list.go index 3a80bafaaa..6ee3da6e58 100644 --- a/pkg/manifestclient/list.go +++ b/pkg/manifestclient/list.go @@ -42,7 +42,7 @@ func (mrt *manifestRoundTripper) listAll(requestInfo *apirequest.RequestInfo) ([ Resource: requestInfo.Resource, } - kind, err := mrt.getKindForResource(gvr) + kind, err := mrt.discoveryReader.getKindForResource(gvr) if err != nil { return nil, fmt.Errorf("unable to determine list kind: %w", err) } diff --git a/pkg/manifestclient/read_roundtripper.go b/pkg/manifestclient/read_roundtripper.go index 4f798e5c27..bb02906321 100644 --- a/pkg/manifestclient/read_roundtripper.go +++ b/pkg/manifestclient/read_roundtripper.go @@ -2,19 +2,14 @@ package manifestclient import ( "bytes" - "embed" "fmt" "io" "io/fs" "net/http" "strconv" "strings" - "sync" "time" - apidiscoveryv2 "k8s.io/api/apidiscovery/v2" - "k8s.io/apimachinery/pkg/util/json" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -36,23 +31,16 @@ type manifestRoundTripper struct { // requestInfoResolver is the same type constructed the same way as the kube-apiserver requestInfoResolver *apirequest.RequestInfoFactory - lock sync.RWMutex - kindForResource map[schema.GroupVersionResource]kindData -} - -type kindData struct { - kind schema.GroupVersionKind - listKind schema.GroupVersionKind - err error + discoveryReader *discoveryReader } -func newReadRoundTripper(content fs.FS) *manifestRoundTripper { +func newReadRoundTripper(content fs.FS, discoveryRoundTripper *discoveryReader) *manifestRoundTripper { return &manifestRoundTripper{ sourceFS: content, requestInfoResolver: server.NewRequestInfoResolver(&server.Config{ LegacyAPIGroupPrefixes: sets.NewString(server.DefaultLegacyAPIPrefix), }), - kindForResource: make(map[schema.GroupVersionResource]kindData), + discoveryReader: discoveryRoundTripper, } } @@ -82,7 +70,7 @@ func (mrt *manifestRoundTripper) RoundTrip(req *http.Request) (*http.Response, e switch requestInfo.Verb { case "get": if isDiscovery { - returnBody, returnErr = mrt.getGroupResourceDiscovery(requestInfo) + returnBody, returnErr = mrt.discoveryReader.getGroupResourceDiscovery(requestInfo) } else { // TODO handle label and field selectors because single item lists are GETs returnBody, returnErr = mrt.get(requestInfo) @@ -168,84 +156,3 @@ func isServerGroupResourceDiscovery(path string) bool { } return parts[0] == "" && parts[1] == "apis" } - -//go:embed default-discovery -var defaultDiscovery embed.FS - -func (mrt *manifestRoundTripper) getKindForResource(gvr schema.GroupVersionResource) (kindData, error) { - mrt.lock.RLock() - kindForGVR, ok := mrt.kindForResource[gvr] - if ok { - defer mrt.lock.RUnlock() - return kindForGVR, kindForGVR.err - } - mrt.lock.RUnlock() - - mrt.lock.Lock() - defer mrt.lock.Unlock() - - kindForGVR, ok = mrt.kindForResource[gvr] - if ok { - return kindForGVR, kindForGVR.err - } - - discoveryPath := "/apis" - if len(gvr.Group) == 0 { - discoveryPath = "/api" - } - discoveryBytes, err := mrt.getGroupResourceDiscovery(&apirequest.RequestInfo{Path: discoveryPath}) - if err != nil { - kindForGVR.err = fmt.Errorf("error reading discovery: %w", err) - mrt.kindForResource[gvr] = kindForGVR - return kindForGVR, kindForGVR.err - } - - discoveryInfo := &apidiscoveryv2.APIGroupDiscoveryList{} - if err := json.Unmarshal(discoveryBytes, discoveryInfo); err != nil { - kindForGVR.err = fmt.Errorf("error unmarshalling discovery: %w", err) - mrt.kindForResource[gvr] = kindForGVR - return kindForGVR, kindForGVR.err - } - - kindForGVR.err = fmt.Errorf("did not find kind for %v\n", gvr) - for _, groupInfo := range discoveryInfo.Items { - if groupInfo.Name != gvr.Group { - continue - } - for _, versionInfo := range groupInfo.Versions { - if versionInfo.Version != gvr.Version { - continue - } - for _, resourceInfo := range versionInfo.Resources { - if resourceInfo.Resource != gvr.Resource { - continue - } - if resourceInfo.ResponseKind == nil { - continue - } - kindForGVR.kind = schema.GroupVersionKind{ - Group: gvr.Group, - Version: gvr.Version, - Kind: resourceInfo.ResponseKind.Kind, - } - if len(resourceInfo.ResponseKind.Group) > 0 { - kindForGVR.kind.Group = resourceInfo.ResponseKind.Group - } - if len(resourceInfo.ResponseKind.Version) > 0 { - kindForGVR.kind.Version = resourceInfo.ResponseKind.Version - } - kindForGVR.listKind = schema.GroupVersionKind{ - Group: kindForGVR.kind.Group, - Version: kindForGVR.kind.Version, - Kind: resourceInfo.ResponseKind.Kind + "List", - } - kindForGVR.err = nil - mrt.kindForResource[gvr] = kindForGVR - return kindForGVR, kindForGVR.err - } - } - } - - mrt.kindForResource[gvr] = kindForGVR - return kindForGVR, kindForGVR.err -} diff --git a/pkg/manifestclient/readwrite_roundtripper.go b/pkg/manifestclient/readwrite_roundtripper.go index c781a41c53..13d01f5330 100644 --- a/pkg/manifestclient/readwrite_roundtripper.go +++ b/pkg/manifestclient/readwrite_roundtripper.go @@ -49,10 +49,11 @@ func NewRoundTripper(mustGatherDir string) *readWriteRoundTripper { } func newReadWriteRoundTripper(sourceFS fs.FS) *readWriteRoundTripper { - return &readWriteRoundTripper{ - readDelegate: newReadRoundTripper(sourceFS), - writeDelegate: newWriteRoundTripper(), - } + rt := &readWriteRoundTripper{} + discoveryReader := newDiscoveryReader(sourceFS) + rt.readDelegate = newReadRoundTripper(sourceFS, discoveryReader) + rt.writeDelegate = newWriteRoundTripper(discoveryReader) + return rt } type readWriteRoundTripper struct { diff --git a/pkg/manifestclient/write_roundtripper.go b/pkg/manifestclient/write_roundtripper.go index a75dd8a4ae..d85d78dd48 100644 --- a/pkg/manifestclient/write_roundtripper.go +++ b/pkg/manifestclient/write_roundtripper.go @@ -28,18 +28,21 @@ type writeTrackingRoundTripper struct { // requestInfoResolver is the same type constructed the same way as the kube-apiserver requestInfoResolver *apirequest.RequestInfoFactory + discoveryReader *discoveryReader + lock sync.RWMutex nextRequestNumber int actionTracker *AllActionsTracker[TrackedSerializedRequest] } -func newWriteRoundTripper() *writeTrackingRoundTripper { +func newWriteRoundTripper(discoveryRoundTripper *discoveryReader) *writeTrackingRoundTripper { return &writeTrackingRoundTripper{ nextRequestNumber: 1, actionTracker: &AllActionsTracker[TrackedSerializedRequest]{}, requestInfoResolver: server.NewRequestInfoResolver(&server.Config{ LegacyAPIGroupPrefixes: sets.NewString(server.DefaultLegacyAPIPrefix), }), + discoveryReader: discoveryRoundTripper, } } @@ -213,8 +216,14 @@ func (mrt *writeTrackingRoundTripper) roundTrip(req *http.Request) ([]byte, erro ret := &unstructured.Unstructured{Object: map[string]interface{}{}} ret.SetName(serializedRequest.ActionMetadata.Name) ret.SetNamespace(serializedRequest.ActionMetadata.Namespace) - if actionHasRuntimeObjectBody { // TODO might be able to do something generally based on discovery if absolutely necessary + if actionHasRuntimeObjectBody { ret.SetGroupVersionKind(bodyObj.GetObjectKind().GroupVersionKind()) + } else { + kindForResource, err := mrt.discoveryReader.getKindForResource(gvr) + if err != nil { + return nil, err + } + ret.SetGroupVersionKind(kindForResource.kind) } retBytes, err := json.Marshal(ret.Object) if err != nil { diff --git a/pkg/manifestclienttest/client_write_test.go b/pkg/manifestclienttest/client_write_test.go index 7c038d37b3..cb59a8a7fc 100644 --- a/pkg/manifestclienttest/client_write_test.go +++ b/pkg/manifestclienttest/client_write_test.go @@ -3,22 +3,26 @@ package manifestclienttest import ( "context" "io/fs" - "k8s.io/apimachinery/pkg/types" + "net/http" "os" "path/filepath" + "testing" "github.com/davecgh/go-spew/spew" "github.com/google/go-cmp/cmp" + configv1 "github.com/openshift/api/config/v1" applyconfigv1 "github.com/openshift/client-go/config/applyconfigurations/config/v1" configclient "github.com/openshift/client-go/config/clientset/versioned" "github.com/openshift/library-go/pkg/manifestclient" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" applymetav1 "k8s.io/client-go/applyconfigurations/meta/v1" + "k8s.io/client-go/dynamic" "k8s.io/client-go/rest" "k8s.io/utils/ptr" - "net/http" - "testing" ) func TestSimpleWritesChecks(t *testing.T) { @@ -194,6 +198,31 @@ func TestSimpleWritesChecks(t *testing.T) { if len(resultingObj.Name) == 0 { t.Fatal(spew.Sdump(resultingObj)) } + + // the dynamic client uses unstructured.UnstructuredJSONScheme decoder, + // which requires type info for decoding. + // TODO: refactor the test to exercise both clients. + dynamicClient, err := dynamic.NewForConfigAndClient(&rest.Config{}, httpClient) + if err != nil { + t.Fatal(err) + } + unstructuredResultingObj, err := dynamicClient.Resource(configv1.GroupVersion.WithResource("featuregates")).Patch( + ctx, + "instance-name", + types.JSONPatchType, + []byte("json-patch"), + metav1.PatchOptions{}, + ) + if err != nil { + t.Fatal(err) + } + resultingObj = &configv1.FeatureGate{} + if err = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredResultingObj.Object, &resultingObj); err != nil { + t.Fatal(err) + } + if len(resultingObj.Name) == 0 { + t.Fatal(spew.Sdump(resultingObj)) + } }, }, {