Spiff expression in "or logic" ("||") is not working correctly in v1.7.0-beta-4

[DimitarVoynov]

trafficstars

Hi @mandelsoft,

we identified that due to the following change: https://github.com/mandelsoft/spiff/commit/f5332183feaf4bbdaa85934b0397bcb6caf737a4 spiff is not working correctly with the following set of yaml files:

1) context-template.yml

...
terraform:
  db:
    name: (( merge || ( terraform.properties.inst_name "db" ) ))
    user: (( merge ))
    password: (( merge ))

  databases:
    - name: (( terraform.db.name ))
      user: (( terraform.db.user ))
      password: (( terraform.db.password ))
    - <<<: (( merge || ~ ))

  properties:
    <<<: (( merge ))
    inst_name: (( merge ))
.....

2. config.yml

---

credentials: (( merge ))

terraform:
  properties:
    inst_name: apps

  db:
    user: (( credentials.rds.appsdb.user ))
    password: ((!appsdb_password))

  databases:
    - name: availability
      password: ((!availability_password))
      user: availability_admin
    - name: xsuaa
      password: ((!xsuaa_password))
      user: xsuaa_admin

3. credentials.yml

---
credentials:
  rds:
    appsdb:
      user: admin

With spiff versions before v1.7.0-beta-4 the line:

name: (( merge || ( terraform.properties.inst_name "db" ) ))

correctly works - i.e. the value after "||" gets resolved to value "appsdb" - thus resulting in a correct merged context.yml file:

terraform:
  databases:
    - name: availability
      password: ((!availability_password))
      user: availability_admin
    - name: xsuaa
      password: ((!xsuaa_password))
      user: xsuaa_admin
    - name: appsdb
      password: ((!appsdb_password))
      user: admin

while starting with version v1.7.0-beta-4 of spiff - the merged yml is wrong:

terraform:
  databases:
    - name: availability
      password: ((!availability_password))
      user: availability_admin
    - name: xsuaa
      password: ((!xsuaa_password))
      user: xsuaa_admin
    - name: xsuaa
      password: ((!xsuaa_password))
      user: xsuaa_admin

To us this looks like a bug or at least - a change that is not backward compatible. Can you please have a look/fix it?

Thanks, Dimitar

[mandelsoft]

Sorry for the delay, I was very busy with other things...

Hmm, the problem is even worse, the list merge does not work anymore, if the base list has more than one entry.

The minimal scenario seems to be:

  effname: (( properties.inst_name "db" ))

  databases:
    - name: (( effname ))
    - <<<: (( merge || ~ ))

  properties:
    <<<: (( merge ))
    inst_name: apps

 properties: 
    inst_name: apps
  
  databases:
    - name: first
    - name: second

[mandelsoft]

This looks very difficult, basically it formerly worked only by accident. There is a severe problem even in earlier releases which just reveals by the mentioned change in your scenario,

[mandelsoft]

Hi @DimitarVoynov, could you test the branch dev for your scenarios, please.

[DimitarVoynov]

Hi @mandelsoft,

building spiff from the dev branch and testing - we didn't face the reported issue in v1.7.0-beta-4. Are you going to make a new release with the fix so it can be consumable?

Thanks, Dimitar

[mandelsoft]

solved with https://github.com/mandelsoft/spiff/releases/tag/v1.7.0-beta-6