environment var array order not being respected

[jordan-da]

When updating an existing workload's environment vars, the order of the vars is not respected. This is important because in kubernetes you are allowed to reference environment vars that were previously defined in other environment vars as per:

Variable references $(VAR_NAME) are expanded using the previously defined environment variables in the container and any service environment variables.
https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.24/#envvar-v1-core

So if the environment var order is non-deterministic, you can not do native environment var expansion.

Example

Initial

"env":  [
  {
    "name": "VAR1",
    "value": "var 1"
  },
  {
    "name": "VAR2",
    "value": "var 2"
  },
]

Updated

"env":  [
  {
    "name": "VAR2",
    "value": "var 2"
  },
  {
    "name": "VAR1",
    "value": "var 1"
  },
]

Result

"env":  [
  {
    "name": "VAR1",
    "value": "var 1"
  },
  {
    "name": "VAR2",
    "value": "var 2"
  },
]

If the order is not maintained, you can never really use $(VAR1) in a subsequent environment var because you have no way to ensure that the var with the reference is defined where it is suppose to be:

Example

Initial

"env":  [
  {
    "name": "VAR1",
    "value": "var 1"
  },
  // whoops, i put this in the wrong place.. let me fix it
  {
    "name": "VAR3",
    "value": "lets expand $(VAR2)"
  },
  {
    "name": "VAR2",
    "value": "var 2"
  },
]

Updated

"env":  [
  {
    "name": "VAR1",
    "value": "var 1"
  },
  {
    "name": "VAR2",
    "value": "var 2"
  },
  // let me update this to the correct position 
  {
    "name": "VAR3",
    "value": "lets expand $(VAR2)"
  },
]

Result

"env":  [
  {
    "name": "VAR1",
    "value": "var 1"
  },
  // metacontroller didn't move it to the correct position
  {
    "name": "VAR3",
    "value": "lets expand $(VAR2)"
  },
  {
    "name": "VAR2",
    "value": "var 2"
  },
]

[grzesuav]

thanks for reporting !

[grzesuav]

responsible code is probably somewhere in https://github.com/metacontroller/metacontroller/blob/master/pkg/dynamic/apply/apply.go

[grzesuav]

@jordan-da Created a test case and narrow down the place when it happens. Current logic indeed does not respect reordering. Need to check if adjusting that won't cause any issues

[grzesuav]

digging a bit further, name is a know merge key, so metacontroller want's to merge entries inside env entry, treating it as listMap (reasoning for this is explained https://metacontroller.github.io/metacontroller/api/apply.html#conventions). The final fix would be switching to SSA (#501).

In case of merging listMaps, metacontrller tries to preserve partial order, that is why the original order (https://github.com/metacontroller/metacontroller/blob/master/pkg/dynamic/apply/apply.go#L156). In this case it does not make sense, as in both cases (value and valueFrom) it makes sense to override, not merge them with existing ones.