elastic / crd-ref-docs

Generates Kubernetes CRD API reference documentation
Apache License 2.0
99 stars 50 forks source link

Test should also fail on Warnings. Warning and wrong Link: Failed to find types Quantity and IntOrString #13

Closed dionysius closed 1 year ago

dionysius commented 3 years ago
  1. The test does not output and test against warnings
  2. I've encountered those two types the generator is unable to find the reference and thus not corretly generating the doc

Patch to represent the issue and directly have a testcase

diff --git a/test.sh b/test.sh
index 46c37d3..941b307 100755
--- a/test.sh
+++ b/test.sh
@@ -29,7 +29,7 @@ trap '[[ $TEMP_DIR ]] && rm -rf "$TEMP_DIR"' EXIT

 (
     cd "$SCRIPT_DIR"
-    go run main.go --log-level=ERROR --source-path="${SCRIPT_DIR}/test" --renderer=asciidoctor --templates-dir="${SCRIPT_DIR}/templates/asciidoctor" --output-path="${TEMP_DIR}/out.asciidoc"
+    go run main.go --log-level=WARN --source-path="${SCRIPT_DIR}/test" --renderer=asciidoctor --templates-dir="${SCRIPT_DIR}/templates/asciidoctor" --output-path="${TEMP_DIR}/out.asciidoc"
     DIFF=$(diff -a -y --suppress-common-lines "${SCRIPT_DIR}/test/expected.asciidoc" "${TEMP_DIR}/out.asciidoc") || true
     if [ "$DIFF" ]; then
         echo "ERROR: outputs differ"
diff --git a/test/api/v1/guestbook_types.go b/test/api/v1/guestbook_types.go
index b8a8279..a754e6d 100644
--- a/test/api/v1/guestbook_types.go
+++ b/test/api/v1/guestbook_types.go
@@ -18,6 +18,8 @@
 package v1

 import (
+       corev1 "k8s.io/api/core/v1"
+       resource "k8s.io/apimachinery/pkg/api/resource"
        metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )

@@ -31,6 +33,10 @@ type GuestbookSpec struct {
        Selector metav1.LabelSelector `json:"selector,omitempty"`
        // Headers contains a list of header items to include in the page
        Headers []GuestbookHeader `json:"headers,omitempty"`
+       // Test for the Quantity type
+       Quantity resource.Quantity `json:"quantity,omitempty"`
+       // Test for the IntOrString type, although it is the field TargetPort inside of ServicePort
+       ServicePort corev1.ServicePort `json:"serviceport,omitempty"`
 }

 // GuestbookEntry defines an entry in a guest book.
diff --git a/test/expected.asciidoc b/test/expected.asciidoc
index d5b7a9e..e427d36 100644
--- a/test/expected.asciidoc
+++ b/test/expected.asciidoc
@@ -107,6 +107,8 @@ GuestbookSpec defines the desired state of Guestbook.
 | *`entries`* __xref:{anchor_prefix}-github-com-elastic-crd-ref-docs-api-v1-guestbookentry[$$GuestbookEntry$$] array__ | Entries contain guest book entries for the page
 | *`selector`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.15/#labelselector-v1-meta[$$LabelSelector$$]__ | Selector selects something
 | *`headers`* __xref:{anchor_prefix}-github-com-elastic-crd-ref-docs-api-v1-guestbookheader[$$GuestbookHeader$$] array__ | Headers contains a list of header items to include in the page
+| *`quantity`* __link:<TODO>__ | Test for the quantity type
+| *`serviceport`* __link:<TODO>__ | Test for the IntOrString type, although it is the field TargetPort inside of ServicePort
 |===

diff --git a/test/go.mod b/test/go.mod
index 6307794..e8997c2 100644
--- a/test/go.mod
+++ b/test/go.mod
@@ -6,6 +6,7 @@ require (
        github.com/go-logr/logr v0.1.0
        github.com/onsi/ginkgo v1.11.0
        github.com/onsi/gomega v1.8.1
+       k8s.io/api v0.17.2
        k8s.io/apimachinery v0.17.2
        k8s.io/client-go v0.17.2
        sigs.k8s.io/controller-runtime v0.5.0

Missing in the patch:

Current test output with the above patch:

$ ./test.sh 
2021-05-21T21:32:46.958+0200    WARN    crd-ref-docs    Failed to find type     {"name": "Quantity", "package": "k8s.io/apimachinery/pkg/api/resource"}
2021-05-21T21:32:47.046+0200    WARN    crd-ref-docs    Failed to find type     {"name": "IntOrString", "package": "k8s.io/apimachinery/pkg/util/intstr"}
ERROR: outputs differ

| *`quantity`* __link:<TODO>__ | Test for the quantity type   | | *`quantity`* __Quantity__ | Test for the Quantity type
| *`serviceport`* __link:<TODO>__ | Test for the IntOrString  | | *`serviceport`* __link:https://kubernetes.io/docs/reference
thbkrkr commented 2 years ago

Sorry for the long response time. I agree that we can change the logging level in the test.sh script. However, I'm not sure about the issue you encountered. Testing the generation with the 2 fields that use external types seems correct to me:

| *`quantity`* __Quantity__ | Test for the Quantity type
| *`serviceport`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.22/#serviceport-v1-core[$$ServicePort$$]__ | Test for the IntOrString type, although it is the field TargetPort inside of ServicePort

The warn message is not very specfic and a bit confusing: Failed to find type. It doesn't say that we are only looking for types in the packages of the current project. Maybe Found external type would be better?

dionysius commented 2 years ago

The logging level is maybe better seen as suggestion, change at your will. Looking at my post my k8s references were still at 1.17. Might be possible that an update fixed it.

What I remember was that it took a while to figure out what was causing it. I had to remove properties one by one until the culprit was found. If you could include the type name causing the issue in the error output at least this would be quicker to debug.

dionysius commented 1 year ago

Closing due to old k8s api, reopen if issue persist with recent version.