-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
⚠️ Fakeclient: Only set TypeMeta for unstructured #2633
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman 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:
Approvers can indicate their approval by writing |
1929682
to
6c89920
Compare
Currently, the fakeclient unconditionally sets metadata for all objects retrieved through it. This causes two problems: * It differs from the behavior of the real client, except for the cached one if `DeepCopy` is enabled and might lead ppl to think that they can rely on `TypeMeta` being populated, which is absolutely not the case * It causes panics for types that have `TypeMeta` defined as pointer, making it impossible to use the client with them This PR changes the behavior to only populate TypeMeta for `unstructured`.
Thx! /lgtm /hold |
/hold cancel |
there was change in behaviour of kubernetes-sigs/controller-runtime - kubernetes-sigs/controller-runtime#2633. This commit updates test to copy TypeMeta in update CreateOrUpdate function. Signed-off-by: Karel Simon <[email protected]>
there was change in behaviour of kubernetes-sigs/controller-runtime - kubernetes-sigs/controller-runtime#2633. This commit updates test to copy TypeMeta in update CreateOrUpdate function. Signed-off-by: Karel Simon <[email protected]>
…7468) * fix(deps): update module sigs.k8s.io/controller-runtime to v0.17.0 * make generate * Stop relying on GVK being set on SCP object TestReconcileStackConfigPolicy_Reconcile fails because with kubernetes-sigs/controller-runtime#2633 the fakeClient does not set the TypeMeta of the StackConfigPolicy object, so its typeMeta.Kind becomes empty but CanBeOwnedBy depends on it. The behavior change to not populate TypeMeta for regular object is to be aligned with the behavior of the real client, see kubernetes-sigs/cluster-api#9956: - APIReader does not set GVK on regular typed objects - Cached reader does not set GVK on regular typed objects (if disableDeepCopy = false, which is NOT default). We could just set the Kind of the StackConfigPolicy object in the tests but it's better to stop relying on GVK being set on the StackConfigPolicy object. --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Thibault Richard <[email protected]>
- Bump controller-runtime to v0.17.0 - Introduce a few minor fixes for new fakeclient behaviour, see: kubernetes-sigs/controller-runtime#2633 Signed-off-by: Simone Tiraboschi <[email protected]>
- Bump controller-runtime to v0.17.0 - Introduce a few minor fixes for new fakeclient behaviour, see: kubernetes-sigs/controller-runtime#2633 Signed-off-by: Simone Tiraboschi <[email protected]>
See kubernetes-sigs/controller-runtime#2633 for details. Updates #12216 Signed-off-by: Tom Proctor <[email protected]>
…ition for annotated services (#12463) Adds a new TailscaleProxyReady condition type for use in corev1.Service conditions. Also switch our CRDs to use metav1.Condition instead of ConnectorCondition. The Go structs are seralized identically, but it updates some descriptions and validation rules. Update k8s controller-tools and controller-runtime deps to fix the documentation generation for metav1.Condition so that it excludes comments and TODOs. Stop expecting the fake client to populate TypeMeta in tests. See kubernetes-sigs/controller-runtime#2633 for details of the change. Finally, make some minor improvements to validation for service hostnames. Fixes #12216 Co-authored-by: Irbe Krumina <[email protected]> Signed-off-by: Tom Proctor <[email protected]>
…ition for annotated services (tailscale#12463) Adds a new TailscaleProxyReady condition type for use in corev1.Service conditions. Also switch our CRDs to use metav1.Condition instead of ConnectorCondition. The Go structs are seralized identically, but it updates some descriptions and validation rules. Update k8s controller-tools and controller-runtime deps to fix the documentation generation for metav1.Condition so that it excludes comments and TODOs. Stop expecting the fake client to populate TypeMeta in tests. See kubernetes-sigs/controller-runtime#2633 for details of the change. Finally, make some minor improvements to validation for service hostnames. Fixes tailscale#12216 Co-authored-by: Irbe Krumina <[email protected]> Signed-off-by: Tom Proctor <[email protected]>
…ition for annotated services (tailscale#12463) Adds a new TailscaleProxyReady condition type for use in corev1.Service conditions. Also switch our CRDs to use metav1.Condition instead of ConnectorCondition. The Go structs are seralized identically, but it updates some descriptions and validation rules. Update k8s controller-tools and controller-runtime deps to fix the documentation generation for metav1.Condition so that it excludes comments and TODOs. Stop expecting the fake client to populate TypeMeta in tests. See kubernetes-sigs/controller-runtime#2633 for details of the change. Finally, make some minor improvements to validation for service hostnames. Fixes tailscale#12216 Co-authored-by: Irbe Krumina <[email protected]> Signed-off-by: Tom Proctor <[email protected]>
- Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package. - Removed `TypeMeta` from expected deployment object when it's compared to structured one retrieved from fake client (kubernetes-sigs/controller-runtime#2633).
- Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Bumped aws-load-balancer-controller to f39ae43121c3 to use latest CRD scheme in e2e tests. - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package. - Removed `TypeMeta` from expected deployment object when it's compared to structured one retrieved from fake client (kubernetes-sigs/controller-runtime#2633).
- Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Bumped aws-load-balancer-controller to f39ae43121c3 to use latest CRD scheme in e2e tests. - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package. - Removed `TypeMeta` from expected deployment object when it's compared to structured one retrieved from fake client (kubernetes-sigs/controller-runtime#2633). - Migrated infrastructure CRD retrieval to the new package `zz_generated.crd-manifests`: - Updated infrastructure CRD for unit tests using `make update-vendored-crds` command.
- Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Bumped aws-load-balancer-controller to f39ae43121c3 to use latest CRD scheme in e2e tests. - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package. - Removed `TypeMeta` from expected deployment object when it's compared to structured one retrieved from fake client (kubernetes-sigs/controller-runtime#2633). - Migrated infrastructure CRD retrieval to the new package `zz_generated.crd-manifests`: - Updated infrastructure CRD for unit tests using `make update-vendored-crds` command.
- Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Bumped aws-load-balancer-controller to f39ae43121c3 to use latest CRD scheme in e2e tests. - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package. - Removed `TypeMeta` from expected deployment object when it's compared to structured one retrieved from fake client (kubernetes-sigs/controller-runtime#2633). - Migrated infrastructure CRD retrieval to the new package `zz_generated.crd-manifests`: - Updated infrastructure CRD for unit tests using `make update-vendored-crds` command. - Updated envtest setup to use the downstream index. - Infrastructure CRD uses CEL functions backported from newer k8s API. - Upstream `envtest` is not ready to use newer CEL function. - Bumped `setup-envtest` to be able to use `--index` flag.
- Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Bumped aws-load-balancer-controller to f39ae43121c3 to use latest CRD scheme in e2e tests. - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package. - Removed `TypeMeta` from expected deployment object when it's compared to structured one retrieved from fake client (kubernetes-sigs/controller-runtime#2633). - Migrated infrastructure CRD retrieval to the new package `zz_generated.crd-manifests`: - Updated infrastructure CRD for unit tests using `make update-vendored-crds` command. - Updated envtest setup to use the downstream index. - Infrastructure CRD uses CEL functions backported from newer k8s API. - Upstream `envtest` is not ready to use newer CEL function. - Bumped `setup-envtest` to be able to use `--index` flag.
#143) - Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Bumped aws-load-balancer-controller to f39ae43121c3 to use latest CRD scheme in e2e tests. - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package. - Removed `TypeMeta` from expected deployment object when it's compared to structured one retrieved from fake client (kubernetes-sigs/controller-runtime#2633). - Migrated infrastructure CRD retrieval to the new package `zz_generated.crd-manifests`: - Updated infrastructure CRD for unit tests using `make update-vendored-crds` command. - Updated envtest setup to use the downstream index. - Infrastructure CRD uses CEL functions backported from newer k8s API. - Upstream `envtest` is not ready to use newer CEL function. - Bumped `setup-envtest` to be able to use `--index` flag.
Currently, the fakeclient unconditionally sets metadata for all objects retrieved through it. This causes two problems:
DeepCopy
is enabled and might lead ppl to think that they can rely onTypeMeta
being populated, which is absolutely not the caseTypeMeta
defined as pointer, making it impossible to use the client with themThis PR changes the behavior to only populate TypeMeta for
unstructured
.