Shebang lines removed in patches during compositions installation with configurations Master/RC-1.14

[haarchri]

What happened?

It appears that Crossplane has been removing shebangs inside patches starting from Master/RC-1.14 versions. It seems like they might be treated as comments and removed from the final string. This issue only arises when you're installing compositions through a configuration package.

https://github.com/haarchri/issue-xp-missing-shebang

+------------------------+-------------------+---------------------+
|                    Version                 |       BUG           |
+------------------------+-------------------+---------------------+
|   crossplane v1.13.2                       |        NO           |
|   universal-crossplane v1.13.2-up.2        |        NO           |
|   crossplane v1.14.0-rc.0.277.gd40146a9    |       YES           |
|   crossplane v1.14.0-rc.0.347.g9ce5007c    |       YES           |
+------------------------+------------------+----------------------+

crossplane v1.13.2 - no issue
universal-crossplane v1.13.2-up.2 - no issue
Please note that I also tested the Universal Crossplane (UXP) version with the ordered deletion feature, as it is the latest available UXP version.

kubectl get composition -o yaml
apiVersion: v1
items:
- apiVersion: apiextensions.crossplane.io/v1
  kind: Composition
  metadata:
    creationTimestamp: "2023-09-15T07:56:44Z"
    generation: 1
    name: xacmedatabases.acme.com
    ownerReferences:
    - apiVersion: pkg.crossplane.io/v1
      blockOwnerDeletion: true
      controller: true
      kind: ConfigurationRevision
      name: test-test-gz
      uid: 30b60caf-85f9-45b0-9168-e04de21017af
    - apiVersion: pkg.crossplane.io/v1
      blockOwnerDeletion: true
      controller: false
      kind: Configuration
      name: test
      uid: 5ebc88db-4c05-4bbd-a76f-996e3c58debf
    resourceVersion: "936"
    uid: e116141c-2ed4-4f1c-a3ff-8c706951a294
  spec:
    compositeTypeRef:
      apiVersion: acme.com/v1
      kind: XAcmeDatabase
    publishConnectionDetailsWithStoreConfigRef:
      name: default
    resources:
    - base:
        apiVersion: nop.crossplane.io/v1alpha1
        kind: NopResource
        spec:
          forProvider:
            fields:
              combineField: cooler
              stringField: cool
          writeConnectionSecretToRef:
            namespace: crossplane-system
      name: NopResource
      patches:
      - combine:
          strategy: string
          string:
            fmt: |
              Content-Type: text/x-shellscript
              #!/bin/bash -xe
              yum update -y
              yum install -y amazon-ssm-agent
              sudo systemctl enable amazon-ssm-agent
              sudo systemctl start amazon-ssm-agent
              # Bootstrap and join the cluster
              /etc/eks/bootstrap.sh --storageGB '%d' --userStr '%s'
          variables:
          - fromFieldPath: spec.storageGB
          - fromFieldPath: spec.userStr
        toFieldPath: spec.forProvider.fields.combineField
        type: CombineFromComposite
      - fromFieldPath: metadata.uid
        toFieldPath: spec.writeConnectionSecretToRef.name
        transforms:
        - string:
            fmt: '%s-nop'
            type: Format
          type: string
        type: FromCompositeFieldPath
      readinessChecks:
      - matchCondition:
          status: "True"
          type: Ready
        type: MatchCondition
    writeConnectionSecretsToNamespace: crossplane-system
kind: List
metadata:
  resourceVersion: ""

crossplane v1.14.0-rc.0.277.gd40146a9 - issue shebang removed
crossplane v1.14.0-rc.0.347.g9ce5007c - issue shebang removed

kubectl get composition -o yaml
apiVersion: v1
items:
- apiVersion: apiextensions.crossplane.io/v1
  kind: Composition
  metadata:
    creationTimestamp: "2023-09-15T07:59:55Z"
    generation: 1
    name: xacmedatabases.acme.com
    ownerReferences:
    - apiVersion: pkg.crossplane.io/v1
      blockOwnerDeletion: true
      controller: true
      kind: ConfigurationRevision
      name: test-test-gz
      uid: 3e58638d-4fc7-45c9-90d7-4ada46899cb8
    - apiVersion: pkg.crossplane.io/v1
      blockOwnerDeletion: true
      controller: false
      kind: Configuration
      name: test
      uid: 7c954cb1-6474-4dab-a524-473f71bbf121
    resourceVersion: "754"
    uid: f1a44095-2d8e-46fc-8736-0e08e9845fd3
  spec:
    compositeTypeRef:
      apiVersion: acme.com/v1
      kind: XAcmeDatabase
    mode: Resources
    publishConnectionDetailsWithStoreConfigRef:
      name: default
    resources:
    - base:
        apiVersion: nop.crossplane.io/v1alpha1
        kind: NopResource
        spec:
          forProvider:
            fields:
              combineField: cooler
              stringField: cool
          writeConnectionSecretToRef:
            namespace: crossplane-system
      name: NopResource
      patches:
      - combine:
          strategy: string
          string:
            fmt: |
              Content-Type: text/x-shellscript
              yum update -y
              yum install -y amazon-ssm-agent
              sudo systemctl enable amazon-ssm-agent
              sudo systemctl start amazon-ssm-agent
              /etc/eks/bootstrap.sh --storageGB '%d' --userStr '%s'
          variables:
          - fromFieldPath: spec.storageGB
          - fromFieldPath: spec.userStr
        toFieldPath: spec.forProvider.fields.combineField
        type: CombineFromComposite
      - fromFieldPath: metadata.uid
        toFieldPath: spec.writeConnectionSecretToRef.name
        transforms:
        - string:
            fmt: '%s-nop'
            type: Format
          type: string
        type: FromCompositeFieldPath
      readinessChecks:
      - matchCondition:
          status: "True"
          type: Ready
        type: MatchCondition
    writeConnectionSecretsToNamespace: crossplane-system
kind: List
metadata:
  resourceVersion: ""

How can we reproduce it?

check my repo: https://github.com/haarchri/issue-xp-missing-shebang prepared all versions

What environment did it happen in?

Crossplane version:

[jbw976]

We should definitely fix this regression before shipping v1.14 🙏

[phisco]

Looks like we are cleaning too much stuff in here, which was introduced by crossplane/crossplane-runtime#504.

[sttts]

Preprocessing YAML looks fishy. Can't we just decode and check that the result has no keys?

[phisco]

@sttts decode on a fully commented file returns an error, "did not find expected node content", which is not easily checkable. Although I agree it would be better not to have to check the content at all, I feel the proposed change won't be so brittle as it's actually checking whether all lines are commented out or separators, which I'd say should be pretty safe.