From d7430e4a889af569ee84632c5cef25d4c808e83c Mon Sep 17 00:00:00 2001 From: David Eads Date: Thu, 3 Oct 2024 16:50:52 -0400 Subject: [PATCH 1/2] refactor dynamic client creation and add testing ones --- .../dynamic_operator_client.go | 27 +++++--------- .../dynamic_staticpod_operator_client.go | 30 ++++------------ .../test_operator_client.go | 35 +++++++++++++++++++ 3 files changed, 50 insertions(+), 42 deletions(-) create mode 100644 pkg/operator/genericoperatorclient/test_operator_client.go diff --git a/pkg/operator/genericoperatorclient/dynamic_operator_client.go b/pkg/operator/genericoperatorclient/dynamic_operator_client.go index 477224b05b..c71d99ee08 100644 --- a/pkg/operator/genericoperatorclient/dynamic_operator_client.go +++ b/pkg/operator/genericoperatorclient/dynamic_operator_client.go @@ -32,11 +32,11 @@ type StaticPodOperatorStatusExtractorFunc func(obj *unstructured.Unstructured, f type OperatorSpecExtractorFunc func(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.OperatorSpecApplyConfiguration, error) type OperatorStatusExtractorFunc func(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.OperatorStatusApplyConfiguration, error) -func newClusterScopedOperatorClient(config *rest.Config, gvr schema.GroupVersionResource, gvk schema.GroupVersionKind, extractApplySpec StaticPodOperatorSpecExtractorFunc, extractApplyStatus StaticPodOperatorStatusExtractorFunc) (*dynamicOperatorClient, dynamicinformer.DynamicSharedInformerFactory, error) { - dynamicClient, err := dynamic.NewForConfig(config) - if err != nil { - return nil, nil, err +func newClusterScopedOperatorClient(dynamicClient dynamic.Interface, gvr schema.GroupVersionResource, gvk schema.GroupVersionKind, instanceName string, extractApplySpec StaticPodOperatorSpecExtractorFunc, extractApplyStatus StaticPodOperatorStatusExtractorFunc) (*dynamicOperatorClient, dynamicinformer.DynamicSharedInformerFactory, error) { + if len(instanceName) < 1 { + return nil, nil, fmt.Errorf("config name cannot be empty") } + client := dynamicClient.Resource(gvr) informers := dynamicinformer.NewDynamicSharedInformerFactory(dynamicClient, 12*time.Hour) @@ -46,6 +46,7 @@ func newClusterScopedOperatorClient(config *rest.Config, gvr schema.GroupVersion gvk: gvk, informer: informer, client: client, + configName: instanceName, extractApplySpec: extractApplySpec, extractApplyStatus: extractApplyStatus, }, informers, nil @@ -82,28 +83,18 @@ func convertOperatorStatusToStaticPodOperatorStatus(extractApplyStatus OperatorS } func NewClusterScopedOperatorClient(config *rest.Config, gvr schema.GroupVersionResource, gvk schema.GroupVersionKind, extractApplySpec OperatorSpecExtractorFunc, extractApplyStatus OperatorStatusExtractorFunc) (v1helpers.OperatorClientWithFinalizers, dynamicinformer.DynamicSharedInformerFactory, error) { - d, informers, err := newClusterScopedOperatorClient(config, gvr, gvk, - convertOperatorSpecToStaticPodOperatorSpec(extractApplySpec), convertOperatorStatusToStaticPodOperatorStatus(extractApplyStatus)) - if err != nil { - return nil, nil, err - } - d.configName = defaultConfigName - return d, informers, nil + return NewClusterScopedOperatorClientWithConfigName(config, gvr, gvk, defaultConfigName, extractApplySpec, extractApplyStatus) } func NewClusterScopedOperatorClientWithConfigName(config *rest.Config, gvr schema.GroupVersionResource, gvk schema.GroupVersionKind, configName string, extractApplySpec OperatorSpecExtractorFunc, extractApplyStatus OperatorStatusExtractorFunc) (v1helpers.OperatorClientWithFinalizers, dynamicinformer.DynamicSharedInformerFactory, error) { - if len(configName) < 1 { - return nil, nil, fmt.Errorf("config name cannot be empty") - } - d, informers, err := newClusterScopedOperatorClient(config, gvr, gvk, - convertOperatorSpecToStaticPodOperatorSpec(extractApplySpec), convertOperatorStatusToStaticPodOperatorStatus(extractApplyStatus)) + dynamicClient, err := dynamic.NewForConfig(config) if err != nil { return nil, nil, err } - d.configName = configName - return d, informers, nil + return newClusterScopedOperatorClient(dynamicClient, gvr, gvk, configName, + convertOperatorSpecToStaticPodOperatorSpec(extractApplySpec), convertOperatorStatusToStaticPodOperatorStatus(extractApplyStatus)) } type dynamicOperatorClient struct { diff --git a/pkg/operator/genericoperatorclient/dynamic_staticpod_operator_client.go b/pkg/operator/genericoperatorclient/dynamic_staticpod_operator_client.go index 77b7824169..3c3680f62e 100644 --- a/pkg/operator/genericoperatorclient/dynamic_staticpod_operator_client.go +++ b/pkg/operator/genericoperatorclient/dynamic_staticpod_operator_client.go @@ -2,8 +2,6 @@ package genericoperatorclient import ( "context" - "time" - applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" "github.com/imdario/mergo" @@ -25,28 +23,12 @@ func NewStaticPodOperatorClient(config *rest.Config, gvr schema.GroupVersionReso if err != nil { return nil, nil, err } - client := dynamicClient.Resource(gvr) - - informers := dynamicinformer.NewDynamicSharedInformerFactory(dynamicClient, 12*time.Hour) - informer := informers.ForResource(gvr) - - return &dynamicStaticPodOperatorClient{ - dynamicOperatorClient: dynamicOperatorClient{ - gvk: gvk, - configName: defaultConfigName, - informer: informer, - client: client, - extractApplySpec: extractApplySpec, - extractApplyStatus: extractApplyStatus, - }, - }, informers, nil -} -type dynamicStaticPodOperatorClient struct { - dynamicOperatorClient + return newClusterScopedOperatorClient(dynamicClient, gvr, gvk, defaultConfigName, + extractApplySpec, extractApplyStatus) } -func (c dynamicStaticPodOperatorClient) GetStaticPodOperatorState() (*operatorv1.StaticPodOperatorSpec, *operatorv1.StaticPodOperatorStatus, string, error) { +func (c dynamicOperatorClient) GetStaticPodOperatorState() (*operatorv1.StaticPodOperatorSpec, *operatorv1.StaticPodOperatorStatus, string, error) { uncastInstance, err := c.informer.Lister().Get("cluster") if err != nil { return nil, nil, "", err @@ -69,7 +51,7 @@ func getStaticPodOperatorStateFromInstance(instance *unstructured.Unstructured) return spec, status, instance.GetResourceVersion(), nil } -func (c dynamicStaticPodOperatorClient) GetStaticPodOperatorStateWithQuorum(ctx context.Context) (*operatorv1.StaticPodOperatorSpec, *operatorv1.StaticPodOperatorStatus, string, error) { +func (c dynamicOperatorClient) GetStaticPodOperatorStateWithQuorum(ctx context.Context) (*operatorv1.StaticPodOperatorSpec, *operatorv1.StaticPodOperatorStatus, string, error) { instance, err := c.client.Get(ctx, "cluster", metav1.GetOptions{}) if err != nil { return nil, nil, "", err @@ -78,7 +60,7 @@ func (c dynamicStaticPodOperatorClient) GetStaticPodOperatorStateWithQuorum(ctx return getStaticPodOperatorStateFromInstance(instance) } -func (c dynamicStaticPodOperatorClient) UpdateStaticPodOperatorSpec(ctx context.Context, resourceVersion string, spec *operatorv1.StaticPodOperatorSpec) (*operatorv1.StaticPodOperatorSpec, string, error) { +func (c dynamicOperatorClient) UpdateStaticPodOperatorSpec(ctx context.Context, resourceVersion string, spec *operatorv1.StaticPodOperatorSpec) (*operatorv1.StaticPodOperatorSpec, string, error) { uncastOriginal, err := c.informer.Lister().Get("cluster") if err != nil { return nil, "", err @@ -103,7 +85,7 @@ func (c dynamicStaticPodOperatorClient) UpdateStaticPodOperatorSpec(ctx context. return retSpec, ret.GetResourceVersion(), nil } -func (c dynamicStaticPodOperatorClient) UpdateStaticPodOperatorStatus(ctx context.Context, resourceVersion string, status *operatorv1.StaticPodOperatorStatus) (*operatorv1.StaticPodOperatorStatus, error) { +func (c dynamicOperatorClient) UpdateStaticPodOperatorStatus(ctx context.Context, resourceVersion string, status *operatorv1.StaticPodOperatorStatus) (*operatorv1.StaticPodOperatorStatus, error) { uncastOriginal, err := c.informer.Lister().Get("cluster") if err != nil { return nil, err diff --git a/pkg/operator/genericoperatorclient/test_operator_client.go b/pkg/operator/genericoperatorclient/test_operator_client.go new file mode 100644 index 0000000000..500e53ba4f --- /dev/null +++ b/pkg/operator/genericoperatorclient/test_operator_client.go @@ -0,0 +1,35 @@ +package genericoperatorclient + +import ( + "github.com/openshift/library-go/pkg/operator/v1helpers" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/dynamic/dynamicinformer" + "k8s.io/client-go/rest" + "net/http" +) + +func NewOperatorClientWithClient(httpClient *http.Client, gvr schema.GroupVersionResource, gvk schema.GroupVersionKind, extractApplySpec OperatorSpecExtractorFunc, extractApplyStatus OperatorStatusExtractorFunc) (v1helpers.OperatorClientWithFinalizers, dynamicinformer.DynamicSharedInformerFactory, error) { + return NewOperatorClientWithConfigNameWithClient(httpClient, gvr, gvk, defaultConfigName, extractApplySpec, extractApplyStatus) + +} + +func NewOperatorClientWithConfigNameWithClient(httpClient *http.Client, gvr schema.GroupVersionResource, gvk schema.GroupVersionKind, configName string, extractApplySpec OperatorSpecExtractorFunc, extractApplyStatus OperatorStatusExtractorFunc) (v1helpers.OperatorClientWithFinalizers, dynamicinformer.DynamicSharedInformerFactory, error) { + dynamicClient, err := dynamic.NewForConfigAndClient(&rest.Config{}, httpClient) + if err != nil { + return nil, nil, err + } + + return newClusterScopedOperatorClient(dynamicClient, gvr, gvk, configName, + convertOperatorSpecToStaticPodOperatorSpec(extractApplySpec), convertOperatorStatusToStaticPodOperatorStatus(extractApplyStatus)) +} + +func NewStaticPodOperatorClientWithConfigNameWithClient(httpClient *http.Client, gvr schema.GroupVersionResource, gvk schema.GroupVersionKind, configName string, extractApplySpec OperatorSpecExtractorFunc, extractApplyStatus OperatorStatusExtractorFunc) (v1helpers.StaticPodOperatorClient, dynamicinformer.DynamicSharedInformerFactory, error) { + dynamicClient, err := dynamic.NewForConfigAndClient(&rest.Config{}, httpClient) + if err != nil { + return nil, nil, err + } + + return newClusterScopedOperatorClient(dynamicClient, gvr, gvk, configName, + convertOperatorSpecToStaticPodOperatorSpec(extractApplySpec), convertOperatorStatusToStaticPodOperatorStatus(extractApplyStatus)) +} From 2d51c5a85aea14b03917d8225b9816247426e884 Mon Sep 17 00:00:00 2001 From: David Eads Date: Fri, 27 Sep 2024 17:41:46 -0400 Subject: [PATCH 2/2] add mutation tracking to the fake client --- pkg/manifestclient/mutation_tracker.go | 168 +++++++++ .../{roundtripper.go => read_roundtripper.go} | 12 +- pkg/manifestclient/readwrite_roundtripper.go | 94 +++++ pkg/manifestclient/write_roundtripper.go | 162 +++++++++ pkg/manifestclienttest/client_test.go | 35 +- pkg/manifestclienttest/client_write_test.go | 334 ++++++++++++++++++ pkg/manifestclienttest/informer_test.go | 2 +- 7 files changed, 775 insertions(+), 32 deletions(-) create mode 100644 pkg/manifestclient/mutation_tracker.go rename pkg/manifestclient/{roundtripper.go => read_roundtripper.go} (93%) create mode 100644 pkg/manifestclient/readwrite_roundtripper.go create mode 100644 pkg/manifestclient/write_roundtripper.go create mode 100644 pkg/manifestclienttest/client_write_test.go diff --git a/pkg/manifestclient/mutation_tracker.go b/pkg/manifestclient/mutation_tracker.go new file mode 100644 index 0000000000..014b9aa390 --- /dev/null +++ b/pkg/manifestclient/mutation_tracker.go @@ -0,0 +1,168 @@ +package manifestclient + +import ( + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" + "sync" +) + +type AllActionsTracker struct { + lock sync.RWMutex + + ActionToTracker map[Action]*ActionTracker +} + +type Action string + +const ( + // this is really a subset of patch, but we treat it separately because it is useful to do so + ActionApply Action = "server-side-apply" + ActionApplyStatus Action = "server-side-apply-status" + ActionUpdate Action = "update" + ActionUpdateStatus Action = "update-status" + ActionCreate Action = "create" + ActionDelete Action = "delete" +) + +type ActionMetadata struct { + Action Action + GVR schema.GroupVersionResource + Namespace string + Name string +} + +type ActionTracker struct { + Action Action + ResourceToTracker map[schema.GroupVersionResource]*ResourceTracker +} + +type ResourceTracker struct { + GVR schema.GroupVersionResource + NamespaceToTracker map[string]*NamespaceTracker +} + +type NamespaceTracker struct { + Namespace string + NameToTracker map[string]*NameTracker +} + +type NameTracker struct { + Name string + SerializedRequests []SerializedRequest +} + +type SerializedRequest struct { + Options []byte + Body []byte +} + +func (a *AllActionsTracker) AddRequest(metadata ActionMetadata, request SerializedRequest) { + a.lock.Lock() + defer a.lock.Unlock() + + if a.ActionToTracker == nil { + a.ActionToTracker = map[Action]*ActionTracker{} + } + if _, ok := a.ActionToTracker[metadata.Action]; !ok { + a.ActionToTracker[metadata.Action] = &ActionTracker{Action: metadata.Action} + } + a.ActionToTracker[metadata.Action].AddRequest(metadata, request) +} + +func (a *AllActionsTracker) ListActions() []Action { + a.lock.Lock() + defer a.lock.Unlock() + + return sets.KeySet(a.ActionToTracker).UnsortedList() +} + +func (a *AllActionsTracker) MutationsForAction(action Action) *ActionTracker { + a.lock.RLock() + defer a.lock.RUnlock() + + return a.ActionToTracker[action] +} + +func (a *AllActionsTracker) MutationsForMetadata(metadata ActionMetadata) []SerializedRequest { + a.lock.RLock() + defer a.lock.RUnlock() + + actionTracker := a.MutationsForAction(metadata.Action) + if actionTracker == nil { + return nil + } + resourceTracker := actionTracker.MutationsForResource(metadata.GVR) + if resourceTracker == nil { + return nil + } + namespaceTracker := resourceTracker.MutationsForNamespace(metadata.Namespace) + if namespaceTracker == nil { + return nil + } + nameTracker := namespaceTracker.MutationsForName(metadata.Name) + if nameTracker == nil { + return nil + } + return nameTracker.SerializedRequests +} + +func (a *ActionTracker) AddRequest(metadata ActionMetadata, request SerializedRequest) { + if a.ResourceToTracker == nil { + a.ResourceToTracker = map[schema.GroupVersionResource]*ResourceTracker{} + } + if _, ok := a.ResourceToTracker[metadata.GVR]; !ok { + a.ResourceToTracker[metadata.GVR] = &ResourceTracker{GVR: metadata.GVR} + } + a.ResourceToTracker[metadata.GVR].AddRequest(metadata, request) +} + +func (a *ActionTracker) ListResources() []schema.GroupVersionResource { + return sets.KeySet(a.ResourceToTracker).UnsortedList() +} + +func (a *ActionTracker) MutationsForResource(gvr schema.GroupVersionResource) *ResourceTracker { + return a.ResourceToTracker[gvr] +} + +func (a *ResourceTracker) AddRequest(metadata ActionMetadata, request SerializedRequest) { + if a.NamespaceToTracker == nil { + a.NamespaceToTracker = map[string]*NamespaceTracker{} + } + if _, ok := a.NamespaceToTracker[metadata.Namespace]; !ok { + a.NamespaceToTracker[metadata.Namespace] = &NamespaceTracker{Namespace: metadata.Namespace} + } + a.NamespaceToTracker[metadata.Namespace].AddRequest(metadata, request) +} + +func (a *ResourceTracker) ListNamespaces() []string { + return sets.KeySet(a.NamespaceToTracker).UnsortedList() +} + +func (a *ResourceTracker) MutationsForNamespace(namespace string) *NamespaceTracker { + return a.NamespaceToTracker[namespace] +} + +func (a *NamespaceTracker) AddRequest(metadata ActionMetadata, request SerializedRequest) { + if a.NameToTracker == nil { + a.NameToTracker = map[string]*NameTracker{} + } + if _, ok := a.NameToTracker[metadata.Name]; !ok { + a.NameToTracker[metadata.Name] = &NameTracker{Name: metadata.Name} + } + a.NameToTracker[metadata.Name].AddRequest(request) +} + +func (a *NamespaceTracker) ListNames() []string { + return sets.KeySet(a.NameToTracker).UnsortedList() +} + +func (a *NamespaceTracker) MutationsForName(name string) *NameTracker { + return a.NameToTracker[name] +} + +func (a *NameTracker) AddRequest(request SerializedRequest) { + if a.SerializedRequests == nil { + a.SerializedRequests = []SerializedRequest{} + } + a.SerializedRequests = append(a.SerializedRequests, request) +} diff --git a/pkg/manifestclient/roundtripper.go b/pkg/manifestclient/read_roundtripper.go similarity index 93% rename from pkg/manifestclient/roundtripper.go rename to pkg/manifestclient/read_roundtripper.go index 649f9a2632..5ba6be931c 100644 --- a/pkg/manifestclient/roundtripper.go +++ b/pkg/manifestclient/read_roundtripper.go @@ -35,21 +35,13 @@ type RawReader interface { fs.ReadDirFS } -func NewTestingRoundTripper(embedFS embed.FS, prefix string) (*manifestRoundTripper, error) { - return newRoundTripper(newPrefixedReader(embedFS, prefix)) -} - -func NewRoundTripper(mustGatherDir string) (*manifestRoundTripper, error) { - return newRoundTripper(newMustGatherReader(mustGatherDir)) -} - -func newRoundTripper(contentReader RawReader) (*manifestRoundTripper, error) { +func newReadRoundTripper(contentReader RawReader) *manifestRoundTripper { return &manifestRoundTripper{ contentReader: contentReader, requestInfoResolver: server.NewRequestInfoResolver(&server.Config{ LegacyAPIGroupPrefixes: sets.NewString(server.DefaultLegacyAPIPrefix), }), - }, nil + } } type prefixedContentReader struct { diff --git a/pkg/manifestclient/readwrite_roundtripper.go b/pkg/manifestclient/readwrite_roundtripper.go new file mode 100644 index 0000000000..9d64fd3eb6 --- /dev/null +++ b/pkg/manifestclient/readwrite_roundtripper.go @@ -0,0 +1,94 @@ +package manifestclient + +import ( + "bytes" + "embed" + "fmt" + "io" + "net/http" +) + +// Enter here and call `NewForConfigAndClient(&rest.Config{}, httpClient)` +func NewHTTPClient(mustGatherDir string) MutationTrackingClient { + mutationTrackingRoundTripper := newReadWriteRoundTripper(newMustGatherReader(mustGatherDir)) + return &mutationTrackingClient{ + httpClient: &http.Client{ + Transport: mutationTrackingRoundTripper, + }, + mutationTrackingRoundTripper: mutationTrackingRoundTripper, + } +} + +// Enter here and call `NewForConfigAndClient(&rest.Config{}, httpClient)` +func NewTestingHTTPClient(embedFS embed.FS, prefix string) MutationTrackingClient { + mutationTrackingRoundTripper := newReadWriteRoundTripper(newPrefixedReader(embedFS, prefix)) + return &mutationTrackingClient{ + httpClient: &http.Client{ + Transport: mutationTrackingRoundTripper, + }, + mutationTrackingRoundTripper: mutationTrackingRoundTripper, + } +} + +func NewTestingRoundTripper(embedFS embed.FS, prefix string) *readWriteRoundTripper { + return newReadWriteRoundTripper(newPrefixedReader(embedFS, prefix)) +} + +func NewRoundTripper(mustGatherDir string) *readWriteRoundTripper { + return newReadWriteRoundTripper(newMustGatherReader(mustGatherDir)) +} + +func newReadWriteRoundTripper(contentReader RawReader) *readWriteRoundTripper { + return &readWriteRoundTripper{ + readDelegate: newReadRoundTripper(contentReader), + writeDelegate: newWriteRoundTripper(), + } +} + +type readWriteRoundTripper struct { + readDelegate *manifestRoundTripper + writeDelegate *writeTrackingRoundTripper +} + +type MutationTrackingRoundTripper interface { + http.RoundTripper + GetMutations() *AllActionsTracker +} + +type mutationTrackingClient struct { + httpClient *http.Client + + mutationTrackingRoundTripper MutationTrackingRoundTripper +} + +func (m mutationTrackingClient) GetHTTPClient() *http.Client { + return m.httpClient +} + +func (m mutationTrackingClient) GetMutations() *AllActionsTracker { + return m.mutationTrackingRoundTripper.GetMutations() +} + +type MutationTrackingClient interface { + GetHTTPClient() *http.Client + GetMutations() *AllActionsTracker +} + +func (rt *readWriteRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + switch req.Method { + case "GET", "HEAD": + return rt.readDelegate.RoundTrip(req) + case "POST", "PUT", "PATCH", "DELETE": + return rt.writeDelegate.RoundTrip(req) + default: + resp := &http.Response{} + resp.StatusCode = http.StatusInternalServerError + resp.Status = http.StatusText(resp.StatusCode) + resp.Body = io.NopCloser(bytes.NewBufferString(fmt.Sprintf("unhandled verb: %q", req.Method))) + return resp, nil + } +} + +func (rt *readWriteRoundTripper) GetMutations() *AllActionsTracker { + return rt.writeDelegate.actionTracker +} diff --git a/pkg/manifestclient/write_roundtripper.go b/pkg/manifestclient/write_roundtripper.go new file mode 100644 index 0000000000..05cbd98306 --- /dev/null +++ b/pkg/manifestclient/write_roundtripper.go @@ -0,0 +1,162 @@ +package manifestclient + +import ( + "bytes" + "fmt" + "io" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + metainternalversionscheme "k8s.io/apimachinery/pkg/apis/meta/internalversion/scheme" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/json" + "k8s.io/apimachinery/pkg/util/sets" + apirequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/apiserver/pkg/server" + "net/http" + "sigs.k8s.io/yaml" +) + +// Saves all mutations for later serialization and/or inspection. +// In the case of updating the same thing multiple times, all mutations are stored and it's up to the caller to decide +// what to do. +type writeTrackingRoundTripper struct { + // requestInfoResolver is the same type constructed the same way as the kube-apiserver + requestInfoResolver *apirequest.RequestInfoFactory + + actionTracker *AllActionsTracker +} + +func newWriteRoundTripper() *writeTrackingRoundTripper { + return &writeTrackingRoundTripper{ + actionTracker: &AllActionsTracker{}, + requestInfoResolver: server.NewRequestInfoResolver(&server.Config{ + LegacyAPIGroupPrefixes: sets.NewString(server.DefaultLegacyAPIPrefix), + }), + } +} + +func (mrt *writeTrackingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + resp := &http.Response{} + + retJSONBytes, err := mrt.roundTrip(req) + if err != nil { + resp.StatusCode = http.StatusInternalServerError + resp.Status = http.StatusText(resp.StatusCode) + resp.Body = io.NopCloser(bytes.NewBufferString(err.Error())) + return resp, nil + } + + resp.StatusCode = http.StatusOK + resp.Status = http.StatusText(resp.StatusCode) + resp.Body = io.NopCloser(bytes.NewReader(retJSONBytes)) + // We always return application/json. Avoid clients expecting proto for built-ins. + // this may or may not work for apply. Guess we'll find out. + resp.Header = make(http.Header) + resp.Header.Set("Content-Type", "application/json") + + return resp, nil +} + +func (mrt *writeTrackingRoundTripper) roundTrip(req *http.Request) ([]byte, error) { + requestInfo, err := mrt.requestInfoResolver.NewRequestInfo(req) + if err != nil { + return nil, fmt.Errorf("failed reading requestInfo: %w", err) + } + + if !requestInfo.IsResourceRequest { + return nil, fmt.Errorf("non-resource requests are not supported by this implementation") + } + if len(requestInfo.Subresource) != 0 && requestInfo.Subresource != "status" { + return nil, fmt.Errorf("subresource %v is not supported by this implementation", requestInfo.Subresource) + } + + var action Action + switch { + case requestInfo.Verb == "create" && len(requestInfo.Subresource) == 0: + action = ActionCreate + case requestInfo.Verb == "update" && len(requestInfo.Subresource) == 0: + action = ActionUpdate + case requestInfo.Verb == "update" && requestInfo.Subresource == "status": + action = ActionUpdateStatus + case requestInfo.Verb == "patch" && req.Header.Get("Content-Type") == string(types.ApplyPatchType) && len(requestInfo.Subresource) == 0: + action = ActionApply + case requestInfo.Verb == "patch" && req.Header.Get("Content-Type") == string(types.ApplyPatchType) && requestInfo.Subresource == "status": + action = ActionApplyStatus + case requestInfo.Verb == "delete" && len(requestInfo.Subresource) == 0: + action = ActionDelete + default: + return nil, fmt.Errorf("verb %v is not supported by this implementation", requestInfo.Verb) + } + + var opts runtime.Object + switch action { + case ActionApply, ActionApplyStatus: + opts = &metav1.PatchOptions{} + case ActionUpdate, ActionUpdateStatus: + opts = &metav1.UpdateOptions{} + case ActionCreate: + opts = &metav1.CreateOptions{} + case ActionDelete: + opts = &metav1.DeleteOptions{} + } + if err := metainternalversionscheme.ParameterCodec.DecodeParameters(req.URL.Query(), metav1.SchemeGroupVersion, opts); err != nil { + return nil, fmt.Errorf("unable to parse query parameters: %w", err) + } + + optionsBytes, err := yaml.Marshal(opts) + if err != nil { + return nil, fmt.Errorf("unable to encode options: %w", err) + } + + bodyContent := []byte{} + if req.Body != nil { + bodyContent, err = io.ReadAll(req.Body) + if err != nil { + return nil, fmt.Errorf("failed to read body: %w", err) + } + } + bodyObj, err := runtime.Decode(unstructured.UnstructuredJSONScheme, bodyContent) + if err != nil { + return nil, fmt.Errorf("unable to decode body: %w", err) + } + bodyYAMLBytes, err := yaml.Marshal(bodyObj.(*unstructured.Unstructured).Object) + if err != nil { + return nil, fmt.Errorf("unable to encode body: %w", err) + } + + serializedRequest := SerializedRequest{ + Options: optionsBytes, + Body: bodyYAMLBytes, + } + metadata := ActionMetadata{ + Action: action, + GVR: schema.GroupVersionResource{ + Group: requestInfo.APIGroup, + Version: requestInfo.APIVersion, + Resource: requestInfo.Resource, + }, + Namespace: requestInfo.Namespace, + Name: requestInfo.Name, + } + if action == ActionCreate { + // in this case, the name isn't in the URL, it's in the body + metadata.Name = bodyObj.(*unstructured.Unstructured).GetName() + } + + mrt.actionTracker.AddRequest(metadata, serializedRequest) + + // returning a value that will probably not cause the wrapping client to fail, but isn't very useful. + // this keeps calling code from depending on the return value. + ret := &unstructured.Unstructured{Object: map[string]interface{}{}} + ret.SetGroupVersionKind(bodyObj.GetObjectKind().GroupVersionKind()) + ret.SetName(bodyObj.(*unstructured.Unstructured).GetName()) + ret.SetNamespace(bodyObj.(*unstructured.Unstructured).GetNamespace()) + retBytes, err := json.Marshal(ret.Object) + if err != nil { + return nil, fmt.Errorf("unable to encode body: %w", err) + } + return retBytes, nil +} diff --git a/pkg/manifestclienttest/client_test.go b/pkg/manifestclienttest/client_test.go index ef70908361..1c36bed8bd 100644 --- a/pkg/manifestclienttest/client_test.go +++ b/pkg/manifestclienttest/client_test.go @@ -161,7 +161,7 @@ func TestSimpleChecks(t *testing.T) { t.Run(roundTripperTest.name, func(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - test.testFn(t, roundTripperTest.getClient()) + test.testFn(t, roundTripperTest.getClient().GetHTTPClient()) }) } }) @@ -171,36 +171,29 @@ func TestSimpleChecks(t *testing.T) { func defaultRoundTrippers(t *testing.T) []*testRoundTrippers { t.Helper() - mustGatherRoundTripper, err := manifestclient.NewRoundTripper("testdata/must-gather-01") - if err != nil { - t.Fatal(err) - } - testRoundTripper, err := manifestclient.NewTestingRoundTripper(mustGather01, "testdata/must-gather-01") - if err != nil { - t.Fatal(err) - } - return []*testRoundTrippers{ { - name: "directory read", - roundTripper: mustGatherRoundTripper, + name: "directory read", + newClientFn: func() manifestclient.MutationTrackingClient { + return manifestclient.NewHTTPClient("testdata/must-gather-01") + }, }, { - name: "embed read", - roundTripper: testRoundTripper, + name: "embed read", + newClientFn: func() manifestclient.MutationTrackingClient { + return manifestclient.NewTestingHTTPClient(mustGather01, "testdata/must-gather-01") + }, }, } } type testRoundTrippers struct { - name string - roundTripper http.RoundTripper + name string + newClientFn func() manifestclient.MutationTrackingClient } -func (r *testRoundTrippers) getClient() *http.Client { - return &http.Client{ - Transport: r.roundTripper, - } +func (r *testRoundTrippers) getClient() manifestclient.MutationTrackingClient { + return r.newClientFn() } func TestWatchChecks(t *testing.T) { @@ -240,7 +233,7 @@ func TestWatchChecks(t *testing.T) { t.Run(roundTripperTest.name, func(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - test.testFn(t, roundTripperTest.getClient()) + test.testFn(t, roundTripperTest.getClient().GetHTTPClient()) }) } }) diff --git a/pkg/manifestclienttest/client_write_test.go b/pkg/manifestclienttest/client_write_test.go new file mode 100644 index 0000000000..7e7cae3602 --- /dev/null +++ b/pkg/manifestclienttest/client_write_test.go @@ -0,0 +1,334 @@ +package manifestclienttest + +import ( + "bytes" + "context" + "k8s.io/utils/ptr" + + "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/runtime/schema" + applymetav1 "k8s.io/client-go/applyconfigurations/meta/v1" + "k8s.io/client-go/rest" + "net/http" + "sigs.k8s.io/yaml" + "testing" +) + +func featureGateYAMLBytesOrDie(obj *configv1.FeatureGate) []byte { + unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + if err != nil { + panic(err) + } + unstructuredObj["apiVersion"] = "config.openshift.io/v1" + unstructuredObj["kind"] = "FeatureGate" + retBytes, err := yaml.Marshal(unstructuredObj) + if err != nil { + panic(err) + } + return retBytes +} + +func apiserverYAMLBytesOrDie(obj *configv1.APIServer) []byte { + unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + if err != nil { + panic(err) + } + unstructuredObj["apiVersion"] = "config.openshift.io/v1" + unstructuredObj["kind"] = "APIServer" + retBytes, err := yaml.Marshal(unstructuredObj) + if err != nil { + panic(err) + } + return retBytes +} +func TestSimpleWritesChecks(t *testing.T) { + tests := []struct { + name string + testFn func(*testing.T, *http.Client) (location manifestclient.ActionMetadata, expectedBodyBytes, expectedOptionsBytes []byte) + }{ + { + name: "CREATE-crd-in-dataset", + testFn: func(t *testing.T, httpClient *http.Client) (manifestclient.ActionMetadata, []byte, []byte) { + configClient, err := configclient.NewForConfigAndClient(&rest.Config{}, httpClient) + if err != nil { + t.Fatal(err) + } + + mutationObj := &configv1.FeatureGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-item", + }, + } + resultingObj, err := configClient.ConfigV1().FeatureGates().Create(context.TODO(), mutationObj, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + if len(resultingObj.Name) == 0 { + t.Fatal(spew.Sdump(resultingObj)) + } + + return manifestclient.ActionMetadata{ + Action: manifestclient.ActionCreate, + GVR: schema.GroupVersionResource{ + Group: "config.openshift.io", + Version: "v1", + Resource: "featuregates", + }, + Namespace: "", + Name: "new-item", + }, + featureGateYAMLBytesOrDie(mutationObj), + []byte("{}\n") + }, + }, + { + name: "CREATE-crd-not-in-dataset", + testFn: func(t *testing.T, httpClient *http.Client) (manifestclient.ActionMetadata, []byte, []byte) { + configClient, err := configclient.NewForConfigAndClient(&rest.Config{}, httpClient) + if err != nil { + t.Fatal(err) + } + + mutationObj := &configv1.APIServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-item", + }, + } + resultingObj, err := configClient.ConfigV1().APIServers().Create(context.TODO(), mutationObj, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + if len(resultingObj.Name) == 0 { + t.Fatal(spew.Sdump(resultingObj)) + } + + return manifestclient.ActionMetadata{ + Action: manifestclient.ActionCreate, + GVR: schema.GroupVersionResource{ + Group: "config.openshift.io", + Version: "v1", + Resource: "apiservers", + }, + Namespace: "", + Name: "new-item", + }, + apiserverYAMLBytesOrDie(mutationObj), + []byte("{}\n") + }, + }, + { + name: "UPDATE-crd-in-dataset", + testFn: func(t *testing.T, httpClient *http.Client) (manifestclient.ActionMetadata, []byte, []byte) { + configClient, err := configclient.NewForConfigAndClient(&rest.Config{}, httpClient) + if err != nil { + t.Fatal(err) + } + + mutationObj := &configv1.FeatureGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-item", + }, + } + resultingObj, err := configClient.ConfigV1().FeatureGates().Update(context.TODO(), mutationObj, metav1.UpdateOptions{}) + if err != nil { + t.Fatal(err) + } + if len(resultingObj.Name) == 0 { + t.Fatal(spew.Sdump(resultingObj)) + } + + return manifestclient.ActionMetadata{ + Action: manifestclient.ActionUpdate, + GVR: schema.GroupVersionResource{ + Group: "config.openshift.io", + Version: "v1", + Resource: "featuregates", + }, + Namespace: "", + Name: "new-item", + }, + featureGateYAMLBytesOrDie(mutationObj), + []byte("{}\n") + }, + }, + { + name: "UPDATE-STATUS-crd-in-dataset-with-options", + testFn: func(t *testing.T, httpClient *http.Client) (manifestclient.ActionMetadata, []byte, []byte) { + configClient, err := configclient.NewForConfigAndClient(&rest.Config{}, httpClient) + if err != nil { + t.Fatal(err) + } + + mutationObj := &configv1.FeatureGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-item", + }, + } + resultingObj, err := configClient.ConfigV1().FeatureGates().UpdateStatus(context.TODO(), mutationObj, metav1.UpdateOptions{FieldValidation: "Strict"}) + if err != nil { + t.Fatal(err) + } + if len(resultingObj.Name) == 0 { + t.Fatal(spew.Sdump(resultingObj)) + } + + return manifestclient.ActionMetadata{ + Action: manifestclient.ActionUpdateStatus, + GVR: schema.GroupVersionResource{ + Group: "config.openshift.io", + Version: "v1", + Resource: "featuregates", + }, + Namespace: "", + Name: "new-item", + }, + featureGateYAMLBytesOrDie(mutationObj), + []byte("fieldValidation: Strict\n") + }, + }, + { + name: "APPLY-crd-in-dataset", + testFn: func(t *testing.T, httpClient *http.Client) (manifestclient.ActionMetadata, []byte, []byte) { + configClient, err := configclient.NewForConfigAndClient(&rest.Config{}, httpClient) + if err != nil { + t.Fatal(err) + } + + applyConfig := applyconfigv1.FeatureGate("new-item") + resultingObj, err := configClient.ConfigV1().FeatureGates().Apply(context.TODO(), applyConfig, metav1.ApplyOptions{ + Force: true, + FieldManager: "the-client", + }) + if err != nil { + t.Fatal(err) + } + if len(resultingObj.Name) == 0 { + t.Fatal(spew.Sdump(resultingObj)) + } + + applyBytes, err := yaml.Marshal(applyConfig) + if err != nil { + t.Fatal(err) + } + + return manifestclient.ActionMetadata{ + Action: manifestclient.ActionApply, + GVR: schema.GroupVersionResource{ + Group: "config.openshift.io", + Version: "v1", + Resource: "featuregates", + }, + Namespace: "", + Name: "new-item", + }, + applyBytes, + []byte("fieldManager: the-client\nforce: true\n") + }, + }, + { + name: "APPLY-STATUS-crd-in-dataset-with-options", + testFn: func(t *testing.T, httpClient *http.Client) (manifestclient.ActionMetadata, []byte, []byte) { + configClient, err := configclient.NewForConfigAndClient(&rest.Config{}, httpClient) + if err != nil { + t.Fatal(err) + } + + applyConfig := applyconfigv1.FeatureGate("new-item"). + WithStatus( + applyconfigv1.FeatureGateStatus(). + WithConditions( + applymetav1.Condition(). + WithType("condition-foo"). + WithStatus(metav1.ConditionTrue), + ), + ) + resultingObj, err := configClient.ConfigV1().FeatureGates().ApplyStatus(context.TODO(), applyConfig, metav1.ApplyOptions{ + Force: true, + FieldManager: "the-client", + }) + if err != nil { + t.Fatal(err) + } + if len(resultingObj.Name) == 0 { + t.Fatal(spew.Sdump(resultingObj)) + } + + applyBytes, err := yaml.Marshal(applyConfig) + if err != nil { + t.Fatal(err) + } + + return manifestclient.ActionMetadata{ + Action: manifestclient.ActionApplyStatus, + GVR: schema.GroupVersionResource{ + Group: "config.openshift.io", + Version: "v1", + Resource: "featuregates", + }, + Namespace: "", + Name: "new-item", + }, + applyBytes, + []byte("fieldManager: the-client\nforce: true\n") + }, + }, + { + name: "DELETE-crd-in-dataset", + testFn: func(t *testing.T, httpClient *http.Client) (manifestclient.ActionMetadata, []byte, []byte) { + configClient, err := configclient.NewForConfigAndClient(&rest.Config{}, httpClient) + if err != nil { + t.Fatal(err) + } + + err = configClient.ConfigV1().FeatureGates().Delete(context.TODO(), "cluster", metav1.DeleteOptions{ + PropagationPolicy: ptr.To(metav1.DeletePropagationForeground), + }) + if err != nil { + t.Fatal(err) + } + + return manifestclient.ActionMetadata{ + Action: manifestclient.ActionDelete, + GVR: schema.GroupVersionResource{ + Group: "config.openshift.io", + Version: "v1", + Resource: "featuregates", + }, + Namespace: "", + Name: "cluster", + }, + []byte("apiVersion: config.openshift.io/v1\nkind: DeleteOptions\npropagationPolicy: Foreground\n"), + []byte("{}\n") + }, + }, + } + + for _, roundTripperTest := range defaultRoundTrippers(t) { + t.Run(roundTripperTest.name, func(t *testing.T) { + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + mutationTrackingClient := roundTripperTest.getClient() + expectedMetadata, expectedBodyBytes, expectedOptionsBytes := test.testFn(t, mutationTrackingClient.GetHTTPClient()) + mutations := mutationTrackingClient.GetMutations() + serializedRequests := mutations.MutationsForMetadata(expectedMetadata) + if len(serializedRequests) != 1 { + t.Fatal(spew.Sdump(mutations)) + } + if !bytes.Equal(serializedRequests[0].Body, expectedBodyBytes) { + t.Fatal(cmp.Diff(string(serializedRequests[0].Body), string(expectedBodyBytes))) + } + if !bytes.Equal(serializedRequests[0].Options, expectedOptionsBytes) { + t.Fatal(cmp.Diff(string(serializedRequests[0].Options), string(expectedOptionsBytes))) + } + }) + } + }) + } +} diff --git a/pkg/manifestclienttest/informer_test.go b/pkg/manifestclienttest/informer_test.go index ec21de7124..e254fc6463 100644 --- a/pkg/manifestclienttest/informer_test.go +++ b/pkg/manifestclienttest/informer_test.go @@ -69,7 +69,7 @@ func TestBasicInformer(t *testing.T) { t.Run(roundTripperTest.name, func(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - test.testFn(t, roundTripperTest.getClient()) + test.testFn(t, roundTripperTest.getClient().GetHTTPClient()) }) } })