🐛 fix provider namespace secret not included in clusterctl move

[archerwu9425]

What this PR does / why we need it:
clusterclt move didn't include secrets in provider namespace, for example:
I have claimed AWSClusterStaticIdentity for cluster A, and add the reference secret to capa-system. But when I tried to use clusterclt move to migrate the cluster, only the AWSClusterStaticIdentity will be included, but the secret for the identity not included and causing issue in target management cluster after move.

The root cause of this is that the discoveryType is Secret but the filter set for provider namespace check is SecretList , so that part of logic will never run into.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Part of #9630

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: archerwu9425 (327b470)

[k8s-ci-robot]

Welcome @archerwu9425!

It looks like this is your first PR to kubernetes-sigs/cluster-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @archerwu9425. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[archerwu9425]

/area area/clusterctl

[k8s-ci-robot]

@archerwu9425: The label(s) area/, area/area/clusterctl cannot be applied, because the repository doesn't have them.

In response to this:

/area area/clusterctl

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[archerwu9425]

/area clusterctl

[killianmuldoon]

/ok-to-test

Thanks for this!

Could we add a unit test to ensure we don't have regression here?

[killianmuldoon]

Note that the linked issue references two problems:

Secrets in the provider namespace should be moved without needing a label.
Secret should not be excluded from move if they have the correct label.

This PR only impacts the first AFAIU, so we should leave the linked issue open to ensure that bug also gets fixed.

[archerwu9425]

/ok-to-test

Thanks for this!

Could we add a unit test to ensure we don't have regression here?

Sure, will add UT

[archerwu9425]

Note that the linked issue references two problems:

Secrets in the provider namespace should be moved without needing a label.
Secret should not be excluded from move if they have the correct label.

This PR only impacts the first AFAIU, so we should leave the linked issue open to ensure that bug also gets fixed.

Per my migration process, the secret in workload cluster namespace with the label is getting migrated successfully, only the provider namespace secret skipped.

[fabriziopandini]

I would like to take a look into the changed behavior for controller runtime fake client so we can decide if to backport/up to which release, but currently dealing with a huge backlog (I will get to this ASAP)

[fabriziopandini]

/hold

[fabriziopandini]

/hold cancel
I went back up to CAPI 1.2 without finding a reason for adding the "List" suffix to the discovered Kinds in our test, all the test are passing also without this.
+1 to remove it, because it was hiding from us a problem in fetching secrets from the provider's namespace which this PR is fixing

/lgtm
@sbueringer PTAL

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 501928021f376a406c6fe64c0560a059b8b0eada

[fabriziopandini]

/cherry-pick release-1.5

[k8s-infra-cherrypick-robot]

@fabriziopandini: once the present PR merges, I will cherry-pick it on top of release-1.5 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[fabriziopandini]

/cherry-pick release-1.4

[k8s-infra-cherrypick-robot]

@fabriziopandini: once the present PR merges, I will cherry-pick it on top of release-1.4 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[sbueringer]

As far as I can tell we check now for "Secret" which we set in

cluster-api/cmd/clusterctl/client/cluster/objectgraph.go

Lines 391 to 392 in 328b4d8

	secretTypeMeta := metav1.TypeMeta{Kind: "Secret", APIVersion: "v1"}
	o.types[getKindAPIString(secretTypeMeta)] = &discoveryTypeInfo{typeMeta: secretTypeMeta}

So this change seems fine to me.

@archerwu9425 Can you please squash the commits?

[archerwu9425]

@sbueringer Squashed. Thanks

[sbueringer]

Thx!

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• cmd/clusterctl/OWNERS [sbueringer]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-infra-cherrypick-robot]

@fabriziopandini: #9694 failed to apply on top of branch "release-1.5":

Applying: remove adding List for discoveryType in test
Using index info to reconstruct a base tree...
M	cmd/clusterctl/client/cluster/mover_test.go
M	cmd/clusterctl/client/cluster/objectgraph.go
M	cmd/clusterctl/client/cluster/objectgraph_test.go
Falling back to patching base and 3-way merge...
Auto-merging cmd/clusterctl/client/cluster/objectgraph_test.go
CONFLICT (content): Merge conflict in cmd/clusterctl/client/cluster/objectgraph_test.go
Auto-merging cmd/clusterctl/client/cluster/objectgraph.go
CONFLICT (content): Merge conflict in cmd/clusterctl/client/cluster/objectgraph.go
Auto-merging cmd/clusterctl/client/cluster/mover_test.go
CONFLICT (content): Merge conflict in cmd/clusterctl/client/cluster/mover_test.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 remove adding List for discoveryType in test
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-infra-cherrypick-robot]

@fabriziopandini: #9694 failed to apply on top of branch "release-1.4":

Applying: remove adding List for discoveryType in test
Using index info to reconstruct a base tree...
M	cmd/clusterctl/client/cluster/mover_test.go
M	cmd/clusterctl/client/cluster/objectgraph.go
M	cmd/clusterctl/client/cluster/objectgraph_test.go
Falling back to patching base and 3-way merge...
Auto-merging cmd/clusterctl/client/cluster/objectgraph_test.go
CONFLICT (content): Merge conflict in cmd/clusterctl/client/cluster/objectgraph_test.go
Auto-merging cmd/clusterctl/client/cluster/objectgraph.go
CONFLICT (content): Merge conflict in cmd/clusterctl/client/cluster/objectgraph.go
Auto-merging cmd/clusterctl/client/cluster/mover_test.go
CONFLICT (content): Merge conflict in cmd/clusterctl/client/cluster/mover_test.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 remove adding List for discoveryType in test
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.