fix: fill defaults for non-required field of type "record"

[nishant95]

Fixes #334

Adds a check for required while recusrively filling defaults for subconfig of type record.
Current behaviour of KIC is out of sync from Kong Lua implementation.

There are some field "required" checks in Lua code: here and here
These checks are missing form go code

[aboudreault]

thanks @nishant95! I just tested your branch and I still have the same issue, my field is set to null. My fix in #333 works but I understand that our PRs don't fix exactly the same thing. To support default with set of record, we'll need to merge both, right?

[nishant95]

@aboudreault Yes, I raised this PR for issue #334 , this deals with record type only.
Your PR looks good and should fix it for type "set".

But shouldn't there be a similar handling for type "map" as well? Similar to this in Lua.

[aboudreault]

@nishant95 this is possible yeah. Let's try to release this fix and #333, I will create a ticket for the map type.

[rainest]

Overriding linter failure and merging cause it ain't worth waiting to round trip to the upstream branch and the error is inscrutable:

Check failure on line 2262 in kong/utils_test.go
GitHub Actions / lint

File is not `gci`-ed with --skip-generated -s standard -s default -s prefix(github.com/kong) (gci)

golangci, please just print the actual command you need to run. Furthermore, the import block didn't change between those commits; it shouldn't start failing. This is not an import >:(

10:47:38-0700 esenin $ gci diff --skip-generated  -s standard -s default -s "prefix(github.com/kong)" utils_test.go 
--- utils_test.go
+++ utils_test.go
@@ -2259,4 +2259,4 @@
 			}
 		})
 	}
-}
\ No newline at end of file
+}

Going to fix in the release changelog commit.

[pmalek]

Is there a reason we want to run this against 3.3 only? If so then I believe this line deserves a comment why that's the case. It's not obvious to the reader why this particular version was chosen here and deserves a special treatment.

[nishant95]

@pmalek You are right, this check is not needed. This is a residual check which I added initially as the schema for this plugin changed a lot over different kong versions, this is of no use now as the schema has been added to the test itself.

[pmalek]

Great! This should fix it then right? #342

[nishant95]

No, this should fix it: #343 Else the test would fail for many of the kong versions as the schema for Acme plugin would be different.