Skip to content

Commit 95138fc

Browse files
cheyangcheyang
authored
fix(transformer): recover the GroupVersionKind of an owner whose TypeMeta is incomplete (#6139)
GenerateOwnerReferenceFromObject read the kind and the apiVersion straight from the object's TypeMeta, which a typed client is allowed to hand back empty. The resulting reference carries an empty kind and "/" as its apiVersion, which the API server rejects, and which an owner based watch cannot resolve back to its owner. The unit tests of the DataLoad value transformers encoded exactly that broken output as their expectation. Recover the missing fields from the GroupVersionKind known by the fluid scheme. The kind and the apiVersion fall back on their own rather than only when the whole TypeMeta is empty, because a partially populated TypeMeta (only the kind, or only the apiVersion, as objects decoded from user YAML can carry) produces a reference which is just as malformed, and because that matches the per-field fallback used for the Dataset owner in #6138. A type which is missing from the scheme is logged, since the function has no error return and the incomplete reference would be invisible otherwise. Objects which do carry a complete TypeMeta are unaffected, so this only changes references which the API server would have rejected anyway. The expectations of the affected tests are fixed accordingly. Two of the chains fed by this helper end up under an owner based watch: DataLoad and DataMigrate render a batchv1.Job whose ownerReference comes from here, and both controllers register Owns(&batchv1.Job{}). Reported by @cheyang while reviewing #6138. Signed-off-by: cheyang <cheyang@163.com>
1 parent f7067fd commit 95138fc

7 files changed

Lines changed: 138 additions & 22 deletions

File tree

‎pkg/ddc/alluxio/load_data_test.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,8 @@ func Test_genDataLoadValue(t *testing.T) {
173173
Name: "test-dataload",
174174
OwnerDatasetId: "fluid-test-dataset",
175175
Owner: &common.OwnerReference{
176-
APIVersion: "/",
176+
Kind: "DataLoad",
177+
APIVersion: "data.fluid.io/v1alpha1",
177178
Enabled: true,
178179
Name: "test-dataload",
179180
BlockOwnerDeletion: false,
@@ -270,7 +271,8 @@ func Test_genDataLoadValue(t *testing.T) {
270271
Name: "test-dataload",
271272
OwnerDatasetId: "fluid-test-dataset",
272273
Owner: &common.OwnerReference{
273-
APIVersion: "/",
274+
Kind: "DataLoad",
275+
APIVersion: "data.fluid.io/v1alpha1",
274276
Enabled: true,
275277
Name: "test-dataload",
276278
BlockOwnerDeletion: false,
@@ -370,7 +372,8 @@ func Test_genDataLoadValue(t *testing.T) {
370372
Name: "test-dataload",
371373
OwnerDatasetId: "fluid-test-dataset",
372374
Owner: &common.OwnerReference{
373-
APIVersion: "/",
375+
Kind: "DataLoad",
376+
APIVersion: "data.fluid.io/v1alpha1",
374377
Enabled: true,
375378
Name: "test-dataload",
376379
BlockOwnerDeletion: false,
@@ -441,7 +444,8 @@ func Test_genDataLoadValue(t *testing.T) {
441444
Name: "test-dataload",
442445
OwnerDatasetId: "fluid-test-dataset",
443446
Owner: &common.OwnerReference{
444-
APIVersion: "/",
447+
Kind: "DataLoad",
448+
APIVersion: "data.fluid.io/v1alpha1",
445449
Enabled: true,
446450
Name: "test-dataload",
447451
BlockOwnerDeletion: false,

‎pkg/ddc/jindo/load_data_test.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,8 @@ func Test_genDataLoadValue(t *testing.T) {
194194
Name: "test-dataload",
195195
OwnerDatasetId: "fluid-test-dataset",
196196
Owner: &common.OwnerReference{
197-
APIVersion: "/",
197+
Kind: "DataLoad",
198+
APIVersion: "data.fluid.io/v1alpha1",
198199
Enabled: true,
199200
Name: "test-dataload",
200201
BlockOwnerDeletion: false,
@@ -307,7 +308,8 @@ func Test_genDataLoadValue(t *testing.T) {
307308
Name: "test-dataload",
308309
OwnerDatasetId: "fluid-test-dataset",
309310
Owner: &common.OwnerReference{
310-
APIVersion: "/",
311+
Kind: "DataLoad",
312+
APIVersion: "data.fluid.io/v1alpha1",
311313
Enabled: true,
312314
Name: "test-dataload",
313315
BlockOwnerDeletion: false,
@@ -423,7 +425,8 @@ func Test_genDataLoadValue(t *testing.T) {
423425
Name: "test-dataload",
424426
OwnerDatasetId: "fluid-test-dataset",
425427
Owner: &common.OwnerReference{
426-
APIVersion: "/",
428+
Kind: "DataLoad",
429+
APIVersion: "data.fluid.io/v1alpha1",
427430
Enabled: true,
428431
Name: "test-dataload",
429432
BlockOwnerDeletion: false,
@@ -510,7 +513,8 @@ func Test_genDataLoadValue(t *testing.T) {
510513
Name: "test-dataload",
511514
OwnerDatasetId: "fluid-test-dataset",
512515
Owner: &common.OwnerReference{
513-
APIVersion: "/",
516+
Kind: "DataLoad",
517+
APIVersion: "data.fluid.io/v1alpha1",
514518
Enabled: true,
515519
Name: "test-dataload",
516520
BlockOwnerDeletion: false,

‎pkg/ddc/jindocache/load_data_test.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,8 @@ func Test_genDataLoadValue(t *testing.T) {
191191
Name: "test-dataload",
192192
OwnerDatasetId: "fluid-test-dataset",
193193
Owner: &common.OwnerReference{
194-
APIVersion: "/",
194+
Kind: "DataLoad",
195+
APIVersion: "data.fluid.io/v1alpha1",
195196
Enabled: true,
196197
Name: "test-dataload",
197198
BlockOwnerDeletion: false,
@@ -304,7 +305,8 @@ func Test_genDataLoadValue(t *testing.T) {
304305
Name: "test-dataload",
305306
OwnerDatasetId: "fluid-test-dataset",
306307
Owner: &common.OwnerReference{
307-
APIVersion: "/",
308+
Kind: "DataLoad",
309+
APIVersion: "data.fluid.io/v1alpha1",
308310
Enabled: true,
309311
Name: "test-dataload",
310312
BlockOwnerDeletion: false,
@@ -420,7 +422,8 @@ func Test_genDataLoadValue(t *testing.T) {
420422
Name: "test-dataload",
421423
OwnerDatasetId: "fluid-test-dataset",
422424
Owner: &common.OwnerReference{
423-
APIVersion: "/",
425+
Kind: "DataLoad",
426+
APIVersion: "data.fluid.io/v1alpha1",
424427
Enabled: true,
425428
Name: "test-dataload",
426429
BlockOwnerDeletion: false,
@@ -507,7 +510,8 @@ func Test_genDataLoadValue(t *testing.T) {
507510
Name: "test-dataload",
508511
OwnerDatasetId: "fluid-test-dataset",
509512
Owner: &common.OwnerReference{
510-
APIVersion: "/",
513+
Kind: "DataLoad",
514+
APIVersion: "data.fluid.io/v1alpha1",
511515
Enabled: true,
512516
Name: "test-dataload",
513517
BlockOwnerDeletion: false,

‎pkg/ddc/jindofsx/load_data_test.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,8 @@ func Test_genDataLoadValue(t *testing.T) {
191191
Name: "test-dataload",
192192
OwnerDatasetId: "fluid-test-dataset",
193193
Owner: &common.OwnerReference{
194-
APIVersion: "/",
194+
Kind: "DataLoad",
195+
APIVersion: "data.fluid.io/v1alpha1",
195196
Enabled: true,
196197
Name: "test-dataload",
197198
BlockOwnerDeletion: false,
@@ -304,7 +305,8 @@ func Test_genDataLoadValue(t *testing.T) {
304305
Name: "test-dataload",
305306
OwnerDatasetId: "fluid-test-dataset",
306307
Owner: &common.OwnerReference{
307-
APIVersion: "/",
308+
Kind: "DataLoad",
309+
APIVersion: "data.fluid.io/v1alpha1",
308310
Enabled: true,
309311
Name: "test-dataload",
310312
BlockOwnerDeletion: false,
@@ -420,7 +422,8 @@ func Test_genDataLoadValue(t *testing.T) {
420422
Name: "test-dataload",
421423
OwnerDatasetId: "fluid-test-dataset",
422424
Owner: &common.OwnerReference{
423-
APIVersion: "/",
425+
Kind: "DataLoad",
426+
APIVersion: "data.fluid.io/v1alpha1",
424427
Enabled: true,
425428
Name: "test-dataload",
426429
BlockOwnerDeletion: false,
@@ -507,7 +510,8 @@ func Test_genDataLoadValue(t *testing.T) {
507510
Name: "test-dataload",
508511
OwnerDatasetId: "fluid-test-dataset",
509512
Owner: &common.OwnerReference{
510-
APIVersion: "/",
513+
Kind: "DataLoad",
514+
APIVersion: "data.fluid.io/v1alpha1",
511515
Enabled: true,
512516
Name: "test-dataload",
513517
BlockOwnerDeletion: false,

‎pkg/ddc/juicefs/data_load_test.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,8 @@ func TestJuiceFSEngine_genDataLoadValue(t *testing.T) {
543543
Name: "test-dataload",
544544
OwnerDatasetId: "fluid-test-dataset",
545545
Owner: &common.OwnerReference{
546-
APIVersion: "/",
546+
Kind: "DataLoad",
547+
APIVersion: "data.fluid.io/v1alpha1",
547548
Enabled: true,
548549
Name: "test-dataload",
549550
BlockOwnerDeletion: false,
@@ -674,7 +675,8 @@ func TestJuiceFSEngine_genDataLoadValue(t *testing.T) {
674675
Name: "test-dataload",
675676
OwnerDatasetId: "fluid-test-dataset",
676677
Owner: &common.OwnerReference{
677-
APIVersion: "/",
678+
Kind: "DataLoad",
679+
APIVersion: "data.fluid.io/v1alpha1",
678680
Enabled: true,
679681
Name: "test-dataload",
680682
BlockOwnerDeletion: false,
@@ -808,7 +810,8 @@ func TestJuiceFSEngine_genDataLoadValue(t *testing.T) {
808810
Name: "test-dataload",
809811
OwnerDatasetId: "fluid-test-dataset",
810812
Owner: &common.OwnerReference{
811-
APIVersion: "/",
813+
Kind: "DataLoad",
814+
APIVersion: "data.fluid.io/v1alpha1",
812815
Enabled: true,
813816
Name: "test-dataload",
814817
BlockOwnerDeletion: false,
@@ -913,7 +916,8 @@ func TestJuiceFSEngine_genDataLoadValue(t *testing.T) {
913916
Name: "test-dataload",
914917
OwnerDatasetId: "fluid-test-dataset",
915918
Owner: &common.OwnerReference{
916-
APIVersion: "/",
919+
Kind: "DataLoad",
920+
APIVersion: "data.fluid.io/v1alpha1",
917921
Enabled: true,
918922
Name: "test-dataload",
919923
BlockOwnerDeletion: false,

‎pkg/utils/transformer/owner_reference.go‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,49 @@ limitations under the License.
1717
package transformer
1818

1919
import (
20+
datav1alpha1 "github.com/fluid-cloudnative/fluid/api/v1alpha1"
2021
"github.com/fluid-cloudnative/fluid/pkg/common"
2122
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
23+
"k8s.io/apimachinery/pkg/runtime"
24+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
25+
ctrl "sigs.k8s.io/controller-runtime"
2226
"sigs.k8s.io/controller-runtime/pkg/client"
27+
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
2328
)
2429

30+
var log = ctrl.Log.WithName("utils.transformer")
31+
32+
// fluidScheme knows the fluid API types. It recovers the GroupVersionKind of an object whose TypeMeta is not
33+
// fully populated, which a typed client is allowed to hand back: an ownerReference missing its kind or its
34+
// apiVersion is rejected by the API server and cannot be resolved back to its owner by an owner-based watch.
35+
var fluidScheme = runtime.NewScheme()
36+
37+
func init() {
38+
utilruntime.Must(datav1alpha1.AddToScheme(fluidScheme))
39+
}
40+
2541
func GenerateOwnerReferenceFromObject(obj client.Object) *common.OwnerReference {
42+
// The kind and the apiVersion fall back on their own, because a partially populated TypeMeta produces a
43+
// reference which is just as malformed as an entirely empty one.
44+
gvk := obj.GetObjectKind().GroupVersionKind()
45+
if len(gvk.Kind) == 0 || len(gvk.Version) == 0 {
46+
resolved, err := apiutil.GVKForObject(obj, fluidScheme)
47+
if err != nil {
48+
log.Error(err, "failed to recover the GroupVersionKind of the owner from the scheme, the generated ownerReference stays incomplete",
49+
"namespace", obj.GetNamespace(), "name", obj.GetName(), "groupVersionKind", gvk.String())
50+
} else {
51+
if len(gvk.Kind) == 0 {
52+
gvk.Kind = resolved.Kind
53+
}
54+
if len(gvk.Version) == 0 {
55+
gvk.Group, gvk.Version = resolved.Group, resolved.Version
56+
}
57+
}
58+
}
2659

2760
ref := &common.OwnerReference{
28-
APIVersion: obj.GetObjectKind().GroupVersionKind().GroupKind().Group + "/" + obj.GetObjectKind().GroupVersionKind().Version,
29-
Kind: obj.GetObjectKind().GroupVersionKind().Kind,
61+
APIVersion: gvk.GroupVersion().String(),
62+
Kind: gvk.Kind,
3063
UID: string(obj.GetUID()),
3164
Enabled: true,
3265
Name: obj.GetName(),

‎pkg/utils/transformer/owner_reference_test.go‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,11 @@ import (
2525

2626
. "github.com/onsi/ginkgo/v2"
2727
. "github.com/onsi/gomega"
28+
corev1 "k8s.io/api/core/v1"
2829
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2930
"k8s.io/apimachinery/pkg/runtime"
3031
"k8s.io/apimachinery/pkg/types"
32+
"sigs.k8s.io/controller-runtime/pkg/client"
3133
)
3234

3335
var _ = Describe("GenerateOwnerReferenceFromObject", func() {
@@ -189,6 +191,67 @@ var _ = Describe("GenerateOwnerReferenceFromObject", func() {
189191
},
190192
),
191193
)
194+
195+
// A typed client is allowed to hand back objects whose TypeMeta is not fully populated, so the kind and
196+
// the apiVersion have to be recovered from the scheme. Otherwise the ownerReference is rejected by the
197+
// API server and cannot be resolved back to its owner by an owner-based watch.
198+
DescribeTable("when the TypeMeta of the object is incomplete",
199+
func(obj client.Object, expectedKind string) {
200+
result := GenerateOwnerReferenceFromObject(obj)
201+
202+
Expect(result.Kind).To(Equal(expectedKind))
203+
Expect(result.APIVersion).To(Equal(datav1alpha1.GroupVersion.String()))
204+
},
205+
206+
Entry("should recover the kind of a dataset",
207+
&datav1alpha1.Dataset{
208+
ObjectMeta: metav1.ObjectMeta{Name: "no-typemeta-dataset", Namespace: "default", UID: "uid-1"},
209+
},
210+
"Dataset",
211+
),
212+
213+
Entry("should recover the kind of an alluxio runtime",
214+
&datav1alpha1.AlluxioRuntime{
215+
ObjectMeta: metav1.ObjectMeta{Name: "no-typemeta-runtime", Namespace: "default", UID: "uid-2"},
216+
},
217+
"AlluxioRuntime",
218+
),
219+
220+
Entry("should recover the kind of a data load",
221+
&datav1alpha1.DataLoad{
222+
ObjectMeta: metav1.ObjectMeta{Name: "no-typemeta-dataload", Namespace: "default", UID: "uid-3"},
223+
},
224+
"DataLoad",
225+
),
226+
227+
Entry("should recover the apiVersion when only the kind is set",
228+
&datav1alpha1.DataLoad{
229+
TypeMeta: metav1.TypeMeta{Kind: "DataLoad"},
230+
ObjectMeta: metav1.ObjectMeta{Name: "kind-only-dataload", Namespace: "default", UID: "uid-4"},
231+
},
232+
"DataLoad",
233+
),
234+
235+
Entry("should recover the kind when only the apiVersion is set",
236+
&datav1alpha1.DataLoad{
237+
TypeMeta: metav1.TypeMeta{APIVersion: datav1alpha1.GroupVersion.String()},
238+
ObjectMeta: metav1.ObjectMeta{Name: "version-only-dataload", Namespace: "default", UID: "uid-5"},
239+
},
240+
"DataLoad",
241+
),
242+
)
243+
244+
It("should leave the reference incomplete for a type the fluid scheme does not know", func() {
245+
// The helper has no error return, so a type missing from the scheme can only be reported through the
246+
// log. Callers all pass registered fluid types today, this only guards against a future one that does
247+
// not.
248+
result := GenerateOwnerReferenceFromObject(&corev1.ConfigMap{
249+
ObjectMeta: metav1.ObjectMeta{Name: "unregistered", Namespace: "default", UID: "uid-6"},
250+
})
251+
252+
Expect(result.Kind).To(BeEmpty())
253+
Expect(result.Name).To(Equal("unregistered"))
254+
})
192255
})
193256

194257
var _ = Describe("FilterOwnerByKind", func() {

0 commit comments

Comments
 (0)