helm-unittest / helm-unittest

BDD styled unit test framework for Kubernetes Helm charts as a Helm plugin.
MIT License
810 stars 256 forks source link

helm-unittest uses different resource ordering than helm #303

Closed raxod502-plaid closed 6 months ago

raxod502-plaid commented 7 months ago

Given https://github.com/helm-unittest/helm-unittest/issues/302, it is still necessary to use documentIndex to select a document from a multi-manifest helm chart. However, this feature is very difficult to use, because the order of resources used by helm-unittest is totally different from the one used by helm (in e.g. helm template).

helm-unittest uses the order of resources exactly as they are specified in the helm chart template files. While helm sorts them according to a hardcoded list of kinds and there is no way to override this, making it difficult to determine the current documentIndex to use, except by guess-and-check or mentally expanding the templates without use of helm.

So for example if you want to write an assertion against resource X in your helm chart, and you run helm template to determine that resource X appears at index 5, say, then when you write a helm-unittest spec with documentIndex: 5 it will probably fail because the assertion is run against some other random resource instead.

ivankatliarchuk commented 7 months ago

Hi, any examples to reproduce?

raxod502-plaid commented 7 months ago

Sure, here is a Chart.yaml:

---
apiVersion: v1
description: Issue 303 for helm-unittest
name: helm-unittest-issue
version: 1.0.0
appVersion: 1.0.0

And templates/manifest.yaml:

---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: example-deployment
---
apiVersion: v1
kind: Service
metadata:
  name: example-service

And tests/_test.yaml:

---
suite: Issue 303 for helm-unittest
tests:
- it: should have the service first
  documentIndex: 0
  asserts:
  - isKind:
      of: Service
  - equal:
      path: metadata.name
      value: example-service
- it: should have the deployment second
  documentIndex: 1
  asserts:
  - isKind:
      of: Deployment
  - equal:
      path: metadata.name
      value: example-deployment

Notice that in the original manifest template, the Deployment is first followed by the Service. But when using helm template then the order is reversed:

% helm template .
---
# Source: helm-unittest-issue/templates/manifest.yaml
apiVersion: v1
kind: Service
metadata:
  name: example-service
---
# Source: helm-unittest-issue/templates/manifest.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: example-deployment

Whereas running helm-unittest fails because it uses the original ordering, while the test is written assuming the Service comes first as in helm template:

% helm unittest .

### Chart [ helm-unittest-issue ] .

 FAIL  Issue 303 for helm-unittest  tests/_test.yaml
    - should have the service first

        - asserts[0] `isKind` fail
            Template:   helm-unittest-issue/templates/manifest.yaml
            DocumentIndex:  0
            Expected to be kind:
                Service
            Actual:
                Deployment

        - asserts[1] `equal` fail
            Template:   helm-unittest-issue/templates/manifest.yaml
            DocumentIndex:  0
            Path:   metadata.name
            Expected to equal:
                example-service
            Actual:
                example-deployment
            Diff:
                --- Expected
                +++ Actual
                @@ -1,2 +1,2 @@
                -example-service
                +example-deployment

    - should have the deployment second

        - asserts[0] `isKind` fail
            Template:   helm-unittest-issue/templates/manifest.yaml
            DocumentIndex:  0
            Expected to be kind:
                Deployment
            Actual:
                Service

        - asserts[1] `equal` fail
            Template:   helm-unittest-issue/templates/manifest.yaml
            DocumentIndex:  0
            Path:   metadata.name
            Expected to equal:
                example-deployment
            Actual:
                example-service
            Diff:
                --- Expected
                +++ Actual
                @@ -1,2 +1,2 @@
                -example-deployment
                +example-service

Charts:      1 failed, 0 passed, 1 total
Test Suites: 1 failed, 0 passed, 1 total
Tests:       2 failed, 0 passed, 2 total
Snapshot:    0 passed, 0 total
Time:        1.75325ms

Error: plugin "unittest" exited with error
ivankatliarchuk commented 6 months ago

Yeah it's a bug. The fix may effect a decent number of helm charts under tests.

Snapshot test example

---
suite: issue 303 for helm-unittest
tests:
- it: manifest should match snapshot
  asserts:
    - matchSnapshot: {}

Snapshot output

manifest should match snapshot:
  1: |
    apiVersion: apps/v1
    kind: Deployment
    metadata:
      name: example-deployment
  2: |
    apiVersion: v1
    kind: Service
    metadata:
      name: example-service

Ordering is not right.

quintush commented 6 months ago

Hello @raxod502-plaid,

The ordering of the helm-chart unittest rendered output is alphabetical. This is a design choice for the following reason.

Where helm is generating in an static order so resources are properly deployed, the helm-unittest does no deployments. Therefore the helm-unittest does not have the static order.

Also when there are multiple the same types (e.g. deployments), the helm template can generate the same types in a different order. In helm-unittest the order has to be always te same, otherwise we will have problems with:

There where previous issues related to inconsistent ordering (#149, #179). This was one of the reasons to develop the documentSelector for those charts where the index of resources is less predictable.

When you need to identify an index value, it is better to use the snapshot feature to identify the outcome and the index order.

Greetings, @quintush

raxod502-plaid commented 6 months ago

That makes sense, I was unaware of the existence of the snapshot feature. It would have been helpful for that to be documented in the section that explained how to use documentIndex, since given the ordering issues, you can't really use documentIndex correctly without also relying on the snapshot feature.

But either way, documentSelector is better in every way, so there is no real need to support documentIndex given that documentSelector supports multiple tests per https://github.com/helm-unittest/helm-unittest/issues/302.