Skip to content

Commit 8191f6c

Browse files
tanmay-dbtanmay-db
authored
Added support for ownership changes for unity catalog resources (#3029)
* - * - * - * - * - * - * working catalog * - * tests * test * - * - * - * - * - * - * - * - * - * - * - * schema * volume * - * - * - * - * - * - * - * - * - * - * - * metastore * - * comments * comments * - * refactor * - * - * comment * - * - * - * - * Make model serving update non breaking * Revert "Add support for Databricks Account Console IP Access Lists (#2586)" This reverts commit 40c92d6. * - * fix * modelServing * - * - * refactor err message * - * comment
1 parent 5d1cedf commit 8191f6c

25 files changed

+2320
-204
lines changed

‎catalog/resource_catalog.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,32 @@ func ResourceCatalog() *schema.Resource {
137137
var updateCatalogRequest catalog.UpdateCatalog
138138
common.DataToStructPointer(d, catalogSchema, &updateCatalogRequest)
139139
updateCatalogRequest.Name = d.Id()
140+
141+
if d.HasChange("owner") {
142+
_, err = w.Catalogs.Update(ctx, catalog.UpdateCatalog{
143+
Name: updateCatalogRequest.Name,
144+
Owner: updateCatalogRequest.Owner,
145+
})
146+
if err != nil {
147+
return err
148+
}
149+
}
150+
151+
updateCatalogRequest.Owner = ""
140152
ci, err := w.Catalogs.Update(ctx, updateCatalogRequest)
153+
141154
if err != nil {
155+
if d.HasChange("owner") {
156+
// Rollback
157+
old, new := d.GetChange("owner")
158+
_, rollbackErr := w.Catalogs.Update(ctx, catalog.UpdateCatalog{
159+
Name: updateCatalogRequest.Name,
160+
Owner: old.(string),
161+
})
162+
if rollbackErr != nil {
163+
return common.OwnerRollbackError(err, rollbackErr, old.(string), new.(string))
164+
}
165+
}
142166
return err
143167
}
144168

‎catalog/resource_catalog_test.go‎

Lines changed: 270 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,6 @@ func TestCatalogCreateWithOwnerAlsoDeletesDefaultSchema(t *testing.T) {
151151
Method: "PATCH",
152152
Resource: "/api/2.1/unity-catalog/catalogs/a",
153153
ExpectedRequest: catalog.UpdateCatalog{
154-
Name: "a",
155154
Comment: "b",
156155
Properties: map[string]string{
157156
"c": "d",
@@ -246,9 +245,20 @@ func TestUpdateCatalog(t *testing.T) {
246245
Method: "PATCH",
247246
Resource: "/api/2.1/unity-catalog/catalogs/a",
248247
ExpectedRequest: catalog.UpdateCatalog{
249-
Name: "a",
248+
Owner: "administrators",
249+
},
250+
Response: catalog.CatalogInfo{
251+
Name: "a",
252+
MetastoreId: "d",
253+
Comment: "c",
254+
Owner: "administrators",
255+
},
256+
},
257+
{
258+
Method: "PATCH",
259+
Resource: "/api/2.1/unity-catalog/catalogs/a",
260+
ExpectedRequest: catalog.UpdateCatalog{
250261
Comment: "c",
251-
Owner: "administrators",
252262
},
253263
Response: catalog.CatalogInfo{
254264
Name: "a",
@@ -284,6 +294,62 @@ func TestUpdateCatalog(t *testing.T) {
284294
}.ApplyNoError(t)
285295
}
286296

297+
func TestUpdateCatalogOwnerOnly(t *testing.T) {
298+
qa.ResourceFixture{
299+
Fixtures: []qa.HTTPFixture{
300+
{
301+
Method: "PATCH",
302+
Resource: "/api/2.1/unity-catalog/catalogs/a",
303+
ExpectedRequest: catalog.UpdateCatalog{
304+
Owner: "updatedOwner",
305+
},
306+
Response: catalog.CatalogInfo{
307+
Name: "a",
308+
MetastoreId: "d",
309+
Comment: "c",
310+
Owner: "updatedOwner",
311+
},
312+
},
313+
{
314+
Method: "PATCH",
315+
Resource: "/api/2.1/unity-catalog/catalogs/a",
316+
ExpectedRequest: catalog.UpdateCatalog{
317+
Comment: "c",
318+
},
319+
Response: catalog.CatalogInfo{
320+
Name: "a",
321+
MetastoreId: "d",
322+
Comment: "c",
323+
Owner: "updatedOwner",
324+
},
325+
},
326+
{
327+
Method: "GET",
328+
Resource: "/api/2.1/unity-catalog/catalogs/a?",
329+
Response: catalog.CatalogInfo{
330+
Name: "a",
331+
MetastoreId: "d",
332+
Comment: "c",
333+
Owner: "updatedOwner",
334+
},
335+
},
336+
},
337+
Resource: ResourceCatalog(),
338+
Update: true,
339+
ID: "a",
340+
InstanceState: map[string]string{
341+
"name": "a",
342+
"comment": "c",
343+
"owner": "administrators",
344+
},
345+
HCL: `
346+
name = "a"
347+
comment = "c"
348+
owner = "updatedOwner"
349+
`,
350+
}.ApplyNoError(t)
351+
}
352+
287353
func TestFailIfMetastoreIdIsWrong(t *testing.T) {
288354
qa.ResourceFixture{
289355
Fixtures: []qa.HTTPFixture{
@@ -323,13 +389,19 @@ func TestUpdateCatalogIfMetastoreIdChanges(t *testing.T) {
323389
MetastoreId: "correct_id",
324390
},
325391
},
392+
{
393+
Method: "PATCH",
394+
Resource: "/api/2.1/unity-catalog/catalogs/a",
395+
ExpectedRequest: catalog.UpdateCatalog{
396+
Owner: "administrators",
397+
},
398+
},
326399
{
327400
Method: "PATCH",
328401
Resource: "/api/2.1/unity-catalog/catalogs/a",
329402
ExpectedRequest: catalog.UpdateCatalog{
330403
Name: "a",
331404
Comment: "c",
332-
Owner: "administrators",
333405
},
334406
Response: catalog.CatalogInfo{
335407
Name: "a",
@@ -380,13 +452,19 @@ func TestUpdateCatalogIfMetastoreIdNotExplicitelySet(t *testing.T) {
380452
MetastoreId: "correct_id",
381453
},
382454
},
455+
{
456+
Method: "PATCH",
457+
Resource: "/api/2.1/unity-catalog/catalogs/a",
458+
ExpectedRequest: catalog.UpdateCatalog{
459+
Owner: "administrators",
460+
},
461+
},
383462
{
384463
Method: "PATCH",
385464
Resource: "/api/2.1/unity-catalog/catalogs/a",
386465
ExpectedRequest: catalog.UpdateCatalog{
387466
Name: "a",
388467
Comment: "c",
389-
Owner: "administrators",
390468
},
391469
Response: catalog.CatalogInfo{
392470
Name: "a",
@@ -417,11 +495,198 @@ func TestUpdateCatalogIfMetastoreIdNotExplicitelySet(t *testing.T) {
417495
HCL: `
418496
name = "a"
419497
comment = "c"
498+
owner = "updatedOwner"
420499
owner = "administrators"
421500
`,
422501
}.ApplyNoError(t)
423502
}
424503

504+
func TestUpdateCatalogOwnerAndOtherFields(t *testing.T) {
505+
qa.ResourceFixture{
506+
Fixtures: []qa.HTTPFixture{
507+
{
508+
Method: "PATCH",
509+
Resource: "/api/2.1/unity-catalog/catalogs/a",
510+
ExpectedRequest: catalog.UpdateCatalog{
511+
Owner: "updatedOwner",
512+
},
513+
Response: catalog.CatalogInfo{
514+
Name: "a",
515+
MetastoreId: "d",
516+
Comment: "c",
517+
Owner: "updatedOwner",
518+
},
519+
},
520+
{
521+
Method: "PATCH",
522+
Resource: "/api/2.1/unity-catalog/catalogs/a",
523+
ExpectedRequest: catalog.UpdateCatalog{
524+
Comment: "e",
525+
},
526+
Response: catalog.CatalogInfo{
527+
Name: "a",
528+
MetastoreId: "d",
529+
Comment: "e",
530+
Owner: "updatedOwner",
531+
},
532+
},
533+
{
534+
Method: "GET",
535+
Resource: "/api/2.1/unity-catalog/catalogs/a?",
536+
Response: catalog.CatalogInfo{
537+
Name: "a",
538+
MetastoreId: "d",
539+
Comment: "e",
540+
Owner: "updatedOwner",
541+
},
542+
},
543+
},
544+
Resource: ResourceCatalog(),
545+
Update: true,
546+
ID: "a",
547+
InstanceState: map[string]string{
548+
"name": "a",
549+
"comment": "c",
550+
"owner": "administrators",
551+
},
552+
HCL: `
553+
name = "a"
554+
comment = "e"
555+
owner = "updatedOwner"
556+
`,
557+
}.ApplyNoError(t)
558+
}
559+
560+
func TestUpdateCatalogUpdateRollback(t *testing.T) {
561+
_, err := qa.ResourceFixture{
562+
Fixtures: []qa.HTTPFixture{
563+
{
564+
Method: "PATCH",
565+
Resource: "/api/2.1/unity-catalog/catalogs/a",
566+
ExpectedRequest: catalog.UpdateCatalog{
567+
Owner: "updatedOwner",
568+
},
569+
Response: catalog.CatalogInfo{
570+
Name: "a",
571+
MetastoreId: "d",
572+
Comment: "c",
573+
Owner: "updatedOwner",
574+
},
575+
},
576+
{
577+
Method: "PATCH",
578+
Resource: "/api/2.1/unity-catalog/catalogs/a",
579+
ExpectedRequest: catalog.UpdateCatalog{
580+
Comment: "e",
581+
},
582+
Response: apierr.APIErrorBody{
583+
ErrorCode: "SERVER_ERROR",
584+
Message: "Something unexpected happened",
585+
},
586+
Status: 500,
587+
},
588+
{
589+
Method: "PATCH",
590+
Resource: "/api/2.1/unity-catalog/catalogs/a",
591+
ExpectedRequest: catalog.UpdateCatalog{
592+
Owner: "administrators",
593+
},
594+
Response: catalog.CatalogInfo{
595+
Name: "a",
596+
MetastoreId: "d",
597+
Comment: "c",
598+
Owner: "administrators",
599+
},
600+
},
601+
{
602+
Method: "GET",
603+
Resource: "/api/2.1/unity-catalog/catalogs/a?",
604+
Response: catalog.CatalogInfo{
605+
Name: "a",
606+
MetastoreId: "d",
607+
Comment: "c",
608+
Owner: "administrators",
609+
},
610+
},
611+
},
612+
Resource: ResourceCatalog(),
613+
Update: true,
614+
ID: "a",
615+
InstanceState: map[string]string{
616+
"name": "a",
617+
"comment": "c",
618+
"owner": "administrators",
619+
},
620+
HCL: `
621+
name = "a"
622+
comment = "e"
623+
owner = "updatedOwner"
624+
`,
625+
}.Apply(t)
626+
qa.AssertErrorStartsWith(t, err, "Something unexpected happened")
627+
}
628+
629+
func TestUpdateCatalogUpdateRollbackError(t *testing.T) {
630+
serverErrMessage := "Something unexpected happened"
631+
rollbackErrMessage := "Internal error happened"
632+
_, err := qa.ResourceFixture{
633+
Fixtures: []qa.HTTPFixture{
634+
{
635+
Method: "PATCH",
636+
Resource: "/api/2.1/unity-catalog/catalogs/a",
637+
ExpectedRequest: catalog.UpdateCatalog{
638+
Owner: "updatedOwner",
639+
},
640+
Response: catalog.CatalogInfo{
641+
Name: "a",
642+
MetastoreId: "d",
643+
Comment: "c",
644+
Owner: "updatedOwner",
645+
},
646+
},
647+
{
648+
Method: "PATCH",
649+
Resource: "/api/2.1/unity-catalog/catalogs/a",
650+
ExpectedRequest: catalog.UpdateCatalog{
651+
Comment: "e",
652+
},
653+
Response: apierr.APIErrorBody{
654+
ErrorCode: "SERVER_ERROR",
655+
Message: serverErrMessage,
656+
},
657+
Status: 500,
658+
},
659+
{
660+
Method: "PATCH",
661+
Resource: "/api/2.1/unity-catalog/catalogs/a",
662+
ExpectedRequest: catalog.UpdateCatalog{
663+
Owner: "administrators",
664+
},
665+
Response: apierr.APIErrorBody{
666+
ErrorCode: "INVALID_REQUEST",
667+
Message: rollbackErrMessage,
668+
},
669+
Status: 400,
670+
},
671+
},
672+
Resource: ResourceCatalog(),
673+
Update: true,
674+
ID: "a",
675+
InstanceState: map[string]string{
676+
"name": "a",
677+
"comment": "c",
678+
"owner": "administrators",
679+
},
680+
HCL: `
681+
name = "a"
682+
comment = "e"
683+
owner = "updatedOwner"
684+
`,
685+
}.Apply(t)
686+
errOccurred := fmt.Sprintf("%s. Owner rollback also failed: %s", serverErrMessage, rollbackErrMessage)
687+
qa.AssertErrorStartsWith(t, err, errOccurred)
688+
}
689+
425690
func TestForceDeleteCatalog(t *testing.T) {
426691
qa.ResourceFixture{
427692
Fixtures: []qa.HTTPFixture{

0 commit comments

Comments
 (0)