Add migration.PlanGeneratorOption to configure a PlanGenerator

[ulucinar]

Description of your changes

Depends on: #153

This PR introduces the migration.PlanGeneratorOption to configure a PlanGenerator in the migrators. Currently, the available configuration options are:

• migration.WithErrorOnInvalidPatchSchema: The migrators will now, by default, not error if issues are encountered while doing a patch schema check against the migration sources & targets and will exclude the patches in the migration target ComposedTemplates after logging the issues. A migrator can revert back to the previous behavior by setting this option on the PlanGenerator.
• migration.WithSkipGVKs: Allows the migrators to configure a list of GVKs that will not be converted during a migration. Wildcard GVKs can be specified by either leaving the API group, version or kind empty. For example, if the kind of a GVK in the skip list is empty, then all kinds in the specified GV will be skipped. List of skipped GVKs override any converters registered with the PlanGenerator's registry, i.e., even if there's a converter registered for a GVK and the GVK is also in the skip list, then the converter will not be run. Resources associated with skipped GVKs are included in the generated migration resources with no further checks or filtering.

This PR also fixes an issue observed when a resource type containing a nested runtime.RawExtension field is being migrated. Migration now continues without any further checks on runtime.RawExtensions. Currently, the framework just traverses the type hierarchy and being able to follow the structure specified by the RawExtension would require us to take the actual values into consideration so initially we just stop checking and filtering at the level a RawExtension is encountered.

I have:

• Read and followed Crossplane's contribution process.
• Run make reviewable to ensure this PR is ready for review.
• Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Manually validated against a migrator converting provider-kubernetes Objects. Objects have a runtime.RawExtension at path spec.forProvider.manifest.

[ytsarev]

• Checked that with migration.WithErrorOnInvalidPatchSchema(false) the migrator under test is reporting errors without the blocking failure:

2023/04/12 22:50:33 Excluding the patch from the migration target because conformance checking has failed with: failed to check whether the patch conforms to the target schema: failed to assert patch schema for path: spec.forProvider.tags.uid: failed to assert source struct field kind at path: uid: type is not a struct:
2023/04/12 22:50:33 Excluding the patch from the migration target because conformance checking has failed with: failed to check whether the patch conforms to the target schema: failed to assert patch schema for path: spec.forProvider.tags.deployment_guid: failed to assert source struct field kind at path: deployment_guid: type is not a struct:
2023/04/12 22:50:33 Excluding the patch from the migration target because conformance checking has failed with: failed to check whether the patch conforms to the target schema: failed to assert patch schema for path: spec.forProvider.tags.app: failed to assert source struct field kind at path: app: type is not a struct:
 echo $?
0

• Tried to skip specific GVK but unfortunately failed
  Skip config:

+       pg := migration.NewPlanGenerator(converters.Registry, source, target, migration.WithErrorOnInvalidPatchSchema(false), migration.WithSkipGVKs(schema.GroupVersionKind{
+               Group:   "ec2.aws.crossplane.io",
+               Kind:    "SecurityGroup",
+               Version: "v1beta1",

The migrated Composition still has

 - base:
      apiVersion: ec2.aws.upbound.io/v1beta1
      kind: SecurityGroup

I added verbosity to associated function

if pg.isGVKSkipped(gvkSource) {
+               fmt.Println("skipping", gvkSource)
                return nil
        }

and pretty sure that it was treated as skipped

go run cmd/migrator/main.go
skipping ec2.aws.crossplane.io/v1beta1, Kind=SecurityGroup
skipping ec2.aws.crossplane.io/v1beta1, Kind=SecurityGroup
skipping ec2.aws.crossplane.io/v1beta1, Kind=SecurityGroup
...

but the migrated SecurityGroup is still in the resulting Composition

[ulucinar]

Thank @ytsarev for giving it a spin. Regarding the migration.WithSkipGVKs, per current implementation, we just skip any patch validation/patch defaulting/conversion logic on the target schema but still include it in the converted composition as an "untouched" ComposedTemplate.

I think what you are expecting as a result of that configuration is to skip any ComposedTemplate with the specified schema (v1.GroupVersionKind) to be excluded in the migration target Compositions. I can modify the PR if this is the desired behavior.

Do we also need the current behavior, where we would like to still keep a target ComposedTemplate but not attempt any patch validation/defaulting or conversion logic on it? Or in our use cases, will it be enough to always skip them in the target Compositions?

[ytsarev]

My expectation is that the target Composition would have an intact source 'ec2.aws.crossplane.io' in this specific example.

More aggressive example is when source Composition has some special resource base, e.g. from some private provider

Consider this to be a part of the source Composition

      - name: customWorkers
        base:
⚠         apiVersion: custom.jet.crossplane.io/v1alpha1
          kind: Aws

The migration will fail with

main: error: Failed to generate the migration plan: failed to generate the migration plan: failed to migrate the composition: failed to migrate the composed templates of the composition: failed to set the defaults on the migration target composed template: failed to instantiate a new source object with GVK: custom.jet.crossplane.io/v1alpha1, Kind=Aws: no kind "Aws" is registered for version "custom.jet.crossplane.io/v1alpha1" in scheme "pkg/runtime/scheme.go:100"

even when we have current skip GVK config of

schema.GroupVersionKind{Group: "jet.crossplane.io"}))

So full conversion skip but keeping the original source config in the target Composition would make sense here.

[ulucinar]

The migration will fail with

main: error: Failed to generate the migration plan: failed to generate the migration plan: failed to migrate the composition: failed to migrate the composed templates of the composition: failed to set the defaults on the migration target composed template: failed to instantiate a new source object with GVK: custom.jet.crossplane.io/v1alpha1, Kind=Aws: no kind "Aws" is registered for version "custom.jet.crossplane.io/v1alpha1" in scheme "pkg/runtime/scheme.go:100"
even when we have current skip GVK config of

schema.GroupVersionKind{Group: "jet.crossplane.io"}))

I think why the skip configuration did not behave as expected is because the API group of the ComposedTemplate is custom.jet.crossplane.io, whereas the skip configuration specifies jet.crossplane.io. If any of the components of a GVK is specified, we need an exact match on them for skipping. A partial postfix won't work.

We can also employ regular expressions here so that the specification language we use can be more powerful.

[ytsarev]

@ulucinar thanks for the clarification!

The switch to exact group specification schema.GroupVersionKind{Group: "custom.jet.crossplane.io"})) helped to configure migrator to skip/ignore the gvk. But in this case, I would love to see it in its original state in the target Composition but it is not there.

In other words, I would expect skipping migration to mean 'transfer the GVK base as it is to target Composition' instead of 'omit the GVK fully'

[ytsarev]

We synced up with @ulucinar and I just double checked the skipping mechanism on my side - it actually transfers the unchanged resource base to the target composition.

So everything behaves as expected, thanks for the amazing job, Alper!

[sergenyalcin]

Thanks @ulucinar LGTM!