test(crd-generator): approval tests for crd-generator-api-v2

[manusa]

Description

Part of #5951 / #5948

Supersedes closes #5978

test(crd-generator): approval tests for crd-generator-api-v2

Relates to:

• [EPIC] CRDGenerator v2 #5948
• Align crd generator v1 and v2 output #5951
• CRDGenerator: Replace assertions with ApprovalTests #5850
• chore(crd-generator): Add approval tests for crd-generator api-v2 #5978
• test(crd-generator): extract approval tests to independent module #5980
• test(crd-generator): refactor approval files #5986
• test(crd-generator): approval tests use lombok for proper serialization #5989

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change
• Chore (non-breaking change which doesn't affect codebase;
  test, version modification, documentation, etc.)

Checklist

• Code contributed by me aligns with current project license: Apache 2.0
• I Added CHANGELOG entry regarding this change
• I have implemented unit tests to cover my changes
• I have added/updated the javadocs and other documentation accordingly
• No new bugs, code smells, etc. in SonarCloud report
• I tested my code in Kubernetes
• I tested my code in OpenShift

[manusa]

Hi @shawkins,

Maybe you can help out here.

This PR adds the tests to ensure feature parity for both versions of the crd-generator.

However, there's a misbehavior, but I'm unsure if this is because something is missing in the new implementation or there's a bug in the old one.

The approval file CRDGeneratorApprovalTest.approvalTest.containingjsons.sample.fabric8.io.v1.approved.yml contains the following lines:

kubernetes-client/crd-generator/test/src/test/resources/io/fabric8/crd/generator/approvaltests/CRDGeneratorApprovalTest.approvalTest.containingjsons.sample.fabric8.io.v1.approved.yml

Lines 22 to 30 in e3f2ae9

	foo:
	properties:
	configAsMap:
	additionalProperties:
	type: "object"
	type: "object"
	x-kubernetes-preserve-unknown-fields: true
	type: "object"
	x-kubernetes-preserve-unknown-fields: true

which are not generated with the new implementation.

This fragment looks instead like:

              foo:
                type: "object"

[shawkins]

@manusa the new one is correct with the default settings. We now have a CRDGenerator property to control whether x-kubernetes-preserve-unknown-fields is implicitly inferred from anyGetter/Setter - it defaults to false. It would need to use the crd annotation for preserve unknown or turn on the implicit setting for this to be seen as x-kubernetes-preserve-unknown-fields.

At the very least, the old one should not have been adding both a configAsMap property and setting x-kubernetes-preserve-unknown-fields - because that was derived from configAsMap.

[manusa]

Thx, yes, it looked more like a bug from the former implementation.
However, I'm not sure it's an easy fix (might break other stuff while fixing it). It might be better to just remove this part from the test (Class).

Thoughts?

/cc @metacosm @andreaTP @baloo42

[baloo42]

IMHO fixing the test for now is better here than changing something in v1 or v2.

Maybe we can add some test cases later to show the differences or a Wiki page to document such things.

[manusa]

👍 I removed the conflicting field + class.
The test case is still valid since it has a JsonNode field that properly maps to x-kubernetes-preserve-unknown-fields: true

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[shawkins]

LGTM