diff --git a/pkg/spoke/controllers/manifestwork_controller.go b/pkg/spoke/controllers/manifestwork_controller.go index ba90844be..d7e859d9c 100644 --- a/pkg/spoke/controllers/manifestwork_controller.go +++ b/pkg/spoke/controllers/manifestwork_controller.go @@ -3,6 +3,7 @@ package controllers import ( "context" "fmt" + "reflect" "strconv" "strings" "time" @@ -28,6 +29,7 @@ import ( "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resource/resourceapply" + "github.com/openshift/library-go/pkg/operator/resource/resourcehelper" "github.com/open-cluster-management/work/pkg/helper" "github.com/open-cluster-management/work/pkg/spoke/resource" @@ -136,35 +138,13 @@ func (m *ManifestWorkController) sync(ctx context.Context, controllerContext fac errs = append(errs, result.Error) } - manifestCondition := workapiv1.ManifestCondition{ - ResourceMeta: workapiv1.ManifestResourceMeta{ - Ordinal: int32(index), - }, - Conditions: []workapiv1.StatusCondition{}, + resourceMeta, err := buildManifestResourceMeta(index, result.Result, m.restMapper) + if err != nil { + errs = append(errs, err) } - - if result.Result != nil && result.Result.GetObjectKind() != nil { - gvk := result.Result.GetObjectKind().GroupVersionKind() - - // set gvk - manifestCondition.ResourceMeta.Group = gvk.Group - manifestCondition.ResourceMeta.Version = gvk.Version - manifestCondition.ResourceMeta.Kind = gvk.Kind - - // set namespace/name - if accessor, err := meta.Accessor(result.Result); err != nil { - errs = append(errs, fmt.Errorf("cannot access metadata of %v: %w", result.Result, err)) - } else { - manifestCondition.ResourceMeta.Namespace = accessor.GetNamespace() - manifestCondition.ResourceMeta.Name = accessor.GetName() - } - - // set resource - if mapping, err := m.restMapper.MappingForGVK(gvk); err != nil { - errs = append(errs, fmt.Errorf("unable to get rest mapping of gvk %v: %w", gvk, err)) - } else if mapping != nil { - manifestCondition.ResourceMeta.Resource = mapping.Resource.Resource - } + manifestCondition := workapiv1.ManifestCondition{ + ResourceMeta: resourceMeta, + Conditions: []workapiv1.StatusCondition{}, } // Add applied status condition @@ -425,3 +405,57 @@ func buildAppliedStatusCondition(err error) workapiv1.StatusCondition { Message: "Apply manifest complete", } } + +func buildManifestResourceMeta(index int, object runtime.Object, restMapper *resource.Mapper) (resourceMeta workapiv1.ManifestResourceMeta, err error) { + resourceMeta = workapiv1.ManifestResourceMeta{ + Ordinal: int32(index), + } + + if object == nil { + return resourceMeta, err + } + + var gvk schema.GroupVersionKind + var namespace, name string + + switch objectType := object.(type) { + case *unstructured.Unstructured: + if objectType == nil { + return resourceMeta, err + } + gvk = objectType.GroupVersionKind() + namespace = objectType.GetNamespace() + name = objectType.GetName() + default: + if reflect.ValueOf(objectType).IsNil() { + return resourceMeta, err + } + gvk = resourcehelper.GuessObjectGroupVersionKind(objectType) + if accessor, e := meta.Accessor(objectType); err != nil { + err = fmt.Errorf("cannot access metadata of %v: %w", objectType, e) + } else { + namespace = accessor.GetNamespace() + name = accessor.GetName() + } + } + + // set gvk + resourceMeta.Group = gvk.Group + resourceMeta.Version = gvk.Version + resourceMeta.Kind = gvk.Kind + + // set namespace/name + resourceMeta.Namespace = namespace + resourceMeta.Name = name + + // set resource + if restMapper == nil { + return resourceMeta, err + } + + if mapping, e := restMapper.MappingForGVK(gvk); e == nil { + resourceMeta.Resource = mapping.Resource.Resource + } + + return resourceMeta, err +} diff --git a/pkg/spoke/controllers/manifestwork_controller_test.go b/pkg/spoke/controllers/manifestwork_controller_test.go index e787c6737..43770b18a 100644 --- a/pkg/spoke/controllers/manifestwork_controller_test.go +++ b/pkg/spoke/controllers/manifestwork_controller_test.go @@ -688,3 +688,62 @@ func TestAllInCondition(t *testing.T) { }) } } + +func TestBuildManifestResourceMeta(t *testing.T) { + var secret *corev1.Secret + var u *unstructured.Unstructured + + cases := []struct { + name string + object runtime.Object + restMapper *resource.Mapper + expected workapiv1.ManifestResourceMeta + }{ + { + name: "build meta for non-unstructured object", + object: newSecret("test", "ns1", "value2"), + expected: workapiv1.ManifestResourceMeta{Version: "v1", Kind: "Secret", Namespace: "ns1", Name: "test"}, + }, + { + name: "build meta for non-unstructured object with rest mapper", + object: newSecret("test", "ns1", "value2"), + restMapper: newFakeMapper(), + expected: workapiv1.ManifestResourceMeta{Version: "v1", Kind: "Secret", Resource: "secrets", Namespace: "ns1", Name: "test"}, + }, + { + name: "build meta for non-unstructured nil", + object: secret, + expected: workapiv1.ManifestResourceMeta{}, + }, + { + name: "build meta for unstructured object", + object: newUnstructured("v1", "Kind1", "ns1", "n1"), + expected: workapiv1.ManifestResourceMeta{Version: "v1", Kind: "Kind1", Namespace: "ns1", Name: "n1"}, + }, + { + name: "build meta for unstructured object with rest mapper", + object: newUnstructured("v1", "NewObject", "ns1", "n1"), + restMapper: newFakeMapper(), + expected: workapiv1.ManifestResourceMeta{Version: "v1", Kind: "NewObject", Resource: "newobjects", Namespace: "ns1", Name: "n1"}, + }, + { + name: "build meta for unstructured nil", + object: u, + expected: workapiv1.ManifestResourceMeta{}, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + actual, err := buildManifestResourceMeta(0, c.object, c.restMapper) + if err != nil { + t.Errorf("Should be success with no err: %v", err) + } + + actual.Ordinal = c.expected.Ordinal + if !equality.Semantic.DeepEqual(actual, c.expected) { + t.Errorf(diff.ObjectDiff(actual, c.expected)) + } + }) + } +}