Adding Database CRD to Glue Controller by JonathanGraniero · Pull Request #16 · aws-controllers-k8s/glue-controller

JonathanGraniero · 2026-02-17T14:25:29Z

Issue #, if available:
No current issue, helping with some missing features

Description of changes:

Adding support for Database CRD. I figured that was probably the best place to start since we need Database and Tables at the minimum for supporting Catalog.

I used #4 as a guide for following the pattern of adding new CRDs but let me know if there is anything different I should do.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

ack-prow · 2026-02-17T14:25:41Z

Hi @JonathanGraniero. Thanks for your PR.

I'm waiting for a aws-controllers-k8s 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.

Details

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.

a-hilaly

Thanks a lot @JonathanGraniero !
I left a few comments in line.

a-hilaly · 2026-03-04T00:06:43Z

/test all

michaelhtm

Great work @JonathanGraniero 💯
left a few comments below
/ok-to-test

michaelhtm · 2026-02-17T19:55:01Z

+	ko := desired.ko.DeepCopy()
+


nit: should we set the Database ARN here?

michaelhtm · 2026-03-04T17:47:52Z

+      Name:
+        is_primary_key: true
+        from:
+          operation: GetDatabase
+          path: Name


this should come from DatabaseInput once unwrapped..
something like this but for input

michaelhtm · 2026-03-04T17:51:40Z

+	// not sure how to determine at this level of abstraction. Probably
+	// something the SDK/runtime should handle. For now, we'll just use
+	// the `aws` partition.
+	return fmt.Sprintf("arn:aws:glue:%s:%s:database/%s", *database.Status.ACKResourceMetadata.Region, *database.Status.ACKResourceMetadata.OwnerAccountID, *database.Spec.Name)


nit: should we nil check any of these>

JonathanGraniero · 2026-03-05T13:22:47Z

Appreciate the quick responses here! I have done all of the updates from both sets of review comments, just doing testing one more time to make sure everything looks good to go.

Apologies for the last one failing codegen, I actually was pushing up to test everything on my other computer and forgot to make it a feature branch 😓

JonathanGraniero · 2026-03-07T00:10:39Z

I think I covered everything including the nits now! Let me know what you think.

michaelhtm · 2026-03-10T16:31:45Z

+      Name:
+        is_primary_key: true
+        from:
+          operation: GetDatabase
+          path: Database.Name


can we mark this as required?
If the input_wrapper_field works we won't need to,
if not, it might be best to mark this as required here

JonathanGraniero · 2026-03-11T20:50:12Z


                  Regex Pattern: `^[\u0020-\uD7FF\uE000-\uFFFD\uD800\uDC00-\uDBFF\uDFFF\t]*$`
                type: string
-              createTableDefaultPermissions:


This is what the API calls it so i'm just changing it to be tableDefaultPermissions which is captured in the gen now

JonathanGraniero · 2026-03-13T19:38:17Z

/retest

JonathanGraniero · 2026-03-13T20:25:34Z

@michaelhtm / @a-hilaly So I have been trying to address this codegen failure. If you look at my past 2 commits, in my current commit, I'm getting (this is just a snippet)

 diff --git a/helm/crds/glue.services.k8s.aws_jobs.yaml b/helm/crds/glue.services.k8s.aws_jobs.yaml
index 2d14f1e..5f46f61 100644
--- a/helm/crds/glue.services.k8s.aws_jobs.yaml
+++ b/helm/crds/glue.services.k8s.aws_jobs.yaml
@@ -2812,11 +2812,11 @@ spec:
                 description: |-
                   A mode that describes how a job was created. Valid values are:
 
-                     * SCRIPT - The job was created using the Glue Studio script editor.
+                    - SCRIPT - The job was created using the Glue Studio script editor.
 
-                     * VISUAL - The job was created using the Glue Studio visual editor.
+                    - VISUAL - The job was created using the Glue Studio visual editor.
 
-                     * NOTEBOOK - The job was created using an interactive sessions notebook.
+                    - NOTEBOOK - The job was created using an interactive sessions notebook.

Where it's saying that it's not matching because of when it's generating these comments the list util is making a - a * but on my previous commit it's saying the opposite:

 diff --git a/config/crd/bases/glue.services.k8s.aws_jobs.yaml b/config/crd/bases/glue.services.k8s.aws_jobs.yaml
index 5f46f61..2d14f1e 100644
--- a/config/crd/bases/glue.services.k8s.aws_jobs.yaml
+++ b/config/crd/bases/glue.services.k8s.aws_jobs.yaml
@@ -2812,11 +2812,11 @@ spec:
                 description: |-
                   A mode that describes how a job was created. Valid values are:
 
-                    - SCRIPT - The job was created using the Glue Studio script editor.
+                     * SCRIPT - The job was created using the Glue Studio script editor.
 
-                    - VISUAL - The job was created using the Glue Studio visual editor.
+                     * VISUAL - The job was created using the Glue Studio visual editor.
 
-                    - NOTEBOOK - The job was created using an interactive sessions notebook.
+                     * NOTEBOOK - The job was created using an interactive sessions notebook.

Am I doing something wrong here?

michaelhtm · 2026-03-25T16:41:02Z

/test glue-verify-code-gen

HansG89 · 2026-08-04T06:29:12Z

Hi, just checking in on this one. Is it still actively being worked on, or blocked on something? We have a use case waiting on this and would be happy to help move it forward if useful. Thanks for the work here!

ack-prow · 2026-08-04T20:32:52Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JonathanGraniero
Once this PR has been reviewed and has the lgtm label, please ask for approval from michaelhtm. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

…, add e2e tests

michaelhtm · 2026-08-05T19:15:37Z

/label release/minor

knottnt · 2026-09-02T18:45:58Z

+	// TODO(a-hilaly): I know there could be other partitions, but I'm
+	// not sure how to determine at this level of abstraction. Probably
+	// something the SDK/runtime should handle. For now, we'll just use
+	// the `aws` partition.


ACK resources now populate a Status.ACKResourceMetadata.Partition that could be used to address this.

knottnt · 2026-09-02T18:50:01Z

+) (*svcsdk.CreateDatabaseInput, error) {
+	res := &svcsdk.CreateDatabaseInput{}
+
+	fw := &svcsdktypes.DatabaseInput{}


CreateDatabase does support setting the initial tags for the resource. Should set tags here instead of deferring to the update path to better support tag based IAM policies.

knottnt · 2026-09-02T19:43:45Z

+
+                  These key-value pairs define parameters and properties of the database.
+                type: object
+              tableDefaultPermissions:


It looks like the Glue service can set a default value for this field post-create which results in repeated spurious deltas in later reconcile loops. Since the CreateDatabase response is empty we'll likely want to mark the field with the below field config to allow for the controller to pickup any default value when it is not initially set.

late_initialize: skip_incomplete_check: {}

ack-prow Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 17, 2026

ack-prow Bot requested review from a-hilaly and michaelhtm February 17, 2026 14:25

ack-prow Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 17, 2026

JonathanGraniero commented Feb 17, 2026

View reviewed changes

Comment thread config/crd/bases/glue.services.k8s.aws_jobs.yaml

JonathanGraniero changed the title ~~adding database crd support~~ Adding Database CRD to Glue Controller Feb 23, 2026

JonathanGraniero marked this pull request as ready for review March 3, 2026 03:00

ack-prow Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 3, 2026

ack-prow Bot requested a review from knottnt March 3, 2026 03:00

a-hilaly reviewed Mar 4, 2026

View reviewed changes

michaelhtm reviewed Mar 4, 2026

View reviewed changes

ack-prow Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 4, 2026

michaelhtm reviewed Mar 10, 2026

View reviewed changes

JonathanGraniero commented Mar 11, 2026

View reviewed changes

JonathanGraniero force-pushed the jg_database_crd_support branch 2 times, most recently from 3a62103 to 272a833 Compare March 12, 2026 00:31

knottnt assigned michaelhtm Aug 4, 2026

michaelhtm force-pushed the jg_database_crd_support branch from 7d977f2 to f6c9ebd Compare August 4, 2026 20:32

feat: add Database resource support

5339f77

michaelhtm force-pushed the jg_database_crd_support branch from f6c9ebd to d77520b Compare August 5, 2026 16:43

michaelhtm force-pushed the jg_database_crd_support branch from d77520b to 7d86d5f Compare August 5, 2026 17:06

fix: late-init CatalogID, immutable Name, correct UpdateDatabase.Name…

2b646a2

…, add e2e tests

michaelhtm force-pushed the jg_database_crd_support branch from 7d86d5f to 2b646a2 Compare August 5, 2026 18:04

ack-prow Bot added the release/minor Indicates this PR should trigger a minor version release on merge. label Aug 5, 2026

This was referenced Aug 19, 2026

Fix glue-verify-code-gen CI: rebase onto main + code-generator ordering fix JonathanGraniero/glue-controller#5

Closed

Fix config/crd/bases vs helm/crds list-marker mismatch (gofmt ordering) aws-controllers-k8s/code-generator#737

Open

Merge branch 'main' into jg_database_crd_support

f981483

knottnt reviewed Sep 2, 2026

View reviewed changes

Uh oh!

Conversation

JonathanGraniero commented Feb 17, 2026

Uh oh!

ack-prow Bot commented Feb 17, 2026

Uh oh!

Uh oh!

a-hilaly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

a-hilaly commented Mar 4, 2026

Uh oh!

michaelhtm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonathanGraniero commented Mar 5, 2026

Uh oh!

JonathanGraniero commented Mar 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonathanGraniero commented Mar 13, 2026

Uh oh!

JonathanGraniero commented Mar 13, 2026

Uh oh!

michaelhtm commented Mar 25, 2026

Uh oh!

HansG89 commented Aug 4, 2026

Uh oh!

ack-prow Bot commented Aug 4, 2026

Uh oh!

michaelhtm commented Aug 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants