Repository: kubernetes-sigs/controller-tools
Pull request #626 (2021-09-29 15:53:39)

🐛 Updating WriteYAML to not add empty line to start files
Currently generated YAML files all have  `\n---\n` prepended to them. Although that's not an issue when using `kubectl apply` with those files, it requires extra work when trying to do something simple like parse CRD yaml from a controller. Although not a significant issue, it's caused us ([Gateway API](https://gateway-api.sigs.k8s.io/) + associated controller authors) to have to workaround it in several places. When parsing YAML files like this, the leading new line is parsed as an empty yaml struct, so we need to write code that can handle this empty/invalid object that starts all our generated YAML CRDs.

It looks like this code originated from https://github.com/kubernetes-sigs/controller-tools/pull/208, and this particular part of the change does not seem like it was intentional.

Initially I wrote this to simply not add the delimiter to the start of the file, but as @howardjohn pointed out, that would make the concatenated files invalid YAML. This change does mean that concatenating YAML files that don't have a trailing new line will not work. Of course all files generated by this tool do have a trailing new line, and that seems like a reasonable expectation of other YAML files.

k8s-ci-robot: @robscott: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-controller-tools-test-master | 56e432703a95ec802d6f2440bed76459555bf05f | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_controller-tools/626/pull-controller-tools-test-master/1443242582485241856) | true | `/test pull-controller-tools-test-master`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=controller-tools&pr=626). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Arobscott). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/controller-tools/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
k8s-ci-robot: @robscott: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-controller-tools-test-master | e1520fcf9e249cccd64f9991b48972467eb36bd8 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_controller-tools/626/pull-controller-tools-test-master/1443243616863522816) | true | `/test pull-controller-tools-test-master`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=controller-tools&pr=626). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Arobscott). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/controller-tools/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
k8s-ci-robot: @robscott: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-controller-tools-test-master | b46655b51d926719eadcd56a2391b7d2347ec602 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_controller-tools/626/pull-controller-tools-test-master/1443244609693028352) | true | `/test pull-controller-tools-test-master`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=controller-tools&pr=626). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3Arobscott). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/controller-tools/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->

Patch:
diff --git a/pkg/genall/genall.go b/pkg/genall/genall.go
index 2cf4ec7e..e03888d4 100644
--- a/pkg/genall/genall.go
+++ b/pkg/genall/genall.go
@@ -135,7 +135,7 @@ func (g GenerationContext) WriteYAML(itemPath string, objs ...interface{}) error
 		if err != nil {
 			return err
 		}
-		n, err := out.Write(append([]byte("\n---\n"), yamlContent...))
+		n, err := out.Write(append([]byte("---\n"), yamlContent...))
 		if err != nil {
 			return err
 		}


Test patch:
diff --git a/pkg/crd/testdata/gen/bar.example.com_foos.v1beta1.yaml b/pkg/crd/testdata/gen/bar.example.com_foos.v1beta1.yaml
index 11b2c7c2..3236c35c 100644
--- a/pkg/crd/testdata/gen/bar.example.com_foos.v1beta1.yaml
+++ b/pkg/crd/testdata/gen/bar.example.com_foos.v1beta1.yaml
@@ -1,4 +1,3 @@
-
 ---
 apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
diff --git a/pkg/crd/testdata/gen/bar.example.com_foos.yaml b/pkg/crd/testdata/gen/bar.example.com_foos.yaml
index 7fe830f2..961c48e1 100644
--- a/pkg/crd/testdata/gen/bar.example.com_foos.yaml
+++ b/pkg/crd/testdata/gen/bar.example.com_foos.yaml
@@ -1,4 +1,3 @@
-
 ---
 apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
diff --git a/pkg/webhook/parser_integration_test.go b/pkg/webhook/parser_integration_test.go
index e9130270..f2f6c438 100644
--- a/pkg/webhook/parser_integration_test.go
+++ b/pkg/webhook/parser_integration_test.go
@@ -178,7 +178,7 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
 })
 
 func unmarshalBothV1(in []byte) (mutating admissionregv1.MutatingWebhookConfiguration, validating admissionregv1.ValidatingWebhookConfiguration) {
-	documents := bytes.Split(in, []byte("\n---\n"))[1:]
+	documents := bytes.Split(in, []byte("\n---\n"))
 	ExpectWithOffset(1, documents).To(HaveLen(2), "expected two documents in file, found %d", len(documents))
 
 	ExpectWithOffset(1, yaml.UnmarshalStrict(documents[0], &mutating)).To(Succeed(), "expected the first document in the file to be a mutating webhook configuration")
diff --git a/pkg/webhook/testdata/invalid-admissionReviewVersionsRequired/manifests.yaml b/pkg/webhook/testdata/invalid-admissionReviewVersionsRequired/manifests.yaml
index 8a579ae9..059794d6 100644
--- a/pkg/webhook/testdata/invalid-admissionReviewVersionsRequired/manifests.yaml
+++ b/pkg/webhook/testdata/invalid-admissionReviewVersionsRequired/manifests.yaml
@@ -1,4 +1,3 @@
-
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
@@ -28,7 +27,6 @@ webhooks:
     resources:
     - cronjobs
   sideEffects: None
-
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
diff --git a/pkg/webhook/testdata/invalid-sideEffects/manifests.yaml b/pkg/webhook/testdata/invalid-sideEffects/manifests.yaml
index 8a579ae9..059794d6 100644
--- a/pkg/webhook/testdata/invalid-sideEffects/manifests.yaml
+++ b/pkg/webhook/testdata/invalid-sideEffects/manifests.yaml
@@ -1,4 +1,3 @@
-
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
@@ -28,7 +27,6 @@ webhooks:
     resources:
     - cronjobs
   sideEffects: None
-
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
diff --git a/pkg/webhook/testdata/invalid-v1beta1NotSupported/manifests.yaml b/pkg/webhook/testdata/invalid-v1beta1NotSupported/manifests.yaml
index 8a579ae9..059794d6 100644
--- a/pkg/webhook/testdata/invalid-v1beta1NotSupported/manifests.yaml
+++ b/pkg/webhook/testdata/invalid-v1beta1NotSupported/manifests.yaml
@@ -1,4 +1,3 @@
-
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
@@ -28,7 +27,6 @@ webhooks:
     resources:
     - cronjobs
   sideEffects: None
-
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
diff --git a/pkg/webhook/testdata/manifests.yaml b/pkg/webhook/testdata/manifests.yaml
index 8a579ae9..059794d6 100644
--- a/pkg/webhook/testdata/manifests.yaml
+++ b/pkg/webhook/testdata/manifests.yaml
@@ -1,4 +1,3 @@
-
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
@@ -28,7 +27,6 @@ webhooks:
     resources:
     - cronjobs
   sideEffects: None
-
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
diff --git a/pkg/webhook/testdata/valid/manifests.yaml b/pkg/webhook/testdata/valid/manifests.yaml
index 8a579ae9..059794d6 100644
--- a/pkg/webhook/testdata/valid/manifests.yaml
+++ b/pkg/webhook/testdata/valid/manifests.yaml
@@ -1,4 +1,3 @@
-
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
@@ -28,7 +27,6 @@ webhooks:
     resources:
     - cronjobs
   sideEffects: None
-
 ---
 apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration