feat: allow destinations to be configured to write straight to the root of the state store

[kirederik]

We recently introduced filepath.mode=none in destinations, which stops kratix from nesting files into directories. However, Kratix will still create at least one directory, named after the destination metadata.name.

There's a good reason why it behaves this way: state stores can be shared across different destinations, and not having a unique top-level directory can lead to file clashes. On the other hand, it's a bit confusing for users to explicitly set it to none but still get "one".

A less surprising experience would be if filepath.mode=none didn't create any directories at all. Users can still create subdirs for the destination by setting the spec.path field. We could also make it configurable via another property.

Warning

Depending on the implementation decision, this may be a breaking change.

Scenarios

Note

There are many ways to implement this story. Update the scenarios accordingly

# Migration for existing Destinatinos
Given a destination set to filepath.mode=none and path=subdir
When I upgrade Kratix to the latest version
Then the migration webhook updates my Destination
And the `spec.path` migrates from `subdir` to `<destination name>/subdir`

# Brand new destination
Given a destination set to filepath.mode=none and path=nil
When a workplacement is created for that destination
Then it's written to the root directory

[aclevername]

+1 to this. When leveraging an existing TFE workspace, its often pointed at the root of a git repo. This means that if I want to use an existing TFE workspace with Kratix, I have to change it to use the sub dir, which is frustrating.

[kirederik]

OK, here's my Formal Proposal, following the guidelines from WikiHow.

The issue

Today, whenever the user create a Destination, Kratix will create a directory with the destination name in the State Store and will push a couple of test documents.

Users have the following levers to configure the path that Kratix uses on the State Store:

• In the State Store itself, there's an optional spec.path field
• In the Destination:
  • there's an optional spec.path field
  • there's a filepath.mode field

For filepath.mode=nestedByMetadata (the default), Kratix will write following a pattern like this:

<StateStore.path> / <Destination.path> / <Promise.name> / <Resource.namespace> /  <Resource.name> / <workflow files>

For filepath.mode=none:

<StateStore.path> / <Destination.name> / <Destination.path> / <workflow files>

However, for some use cases like integrating with existing systems or terraform, users want to define the full path themselves, sometimes even writing to the root of the state store altogether.

That means we need to provide a way for them to not end up with the destination name on the path.

The Solution

Currently, the Destination object looks like this:

apiVersion: platform.kratix.io/v1alpha1
kind: Destination
metadata:
  name: destination-name
  labels:
    environment: dev
spec:
  path: path/in/statestore # optional
  strictMatchLabels: false
  filepath:
    mode: nestedByMetadata | none
  cleanup: none
  stateStoreRef:
    name: default
    kind: BucketStateStore

The first thing that comes to mind as a solution is to just get rid of the destination name from the path entirely.

The danger of this solution is that it may lead users to errors, since if you register two destinations on the same state store, but forget to define different spec.path on the destinations, the documents will all be written to the root directory.

Note

We could create a Validation webhook that checks all the destinations writing to the same state store, and fail (or warn) if we detect possible conflicts.

This is a breaking change that's quite difficult to circumvent. We would need to patch destinations existing prior to the upgrade, but somehow know that, for newer destinations, that could be empty.

An alternative solution is to make spec.path mandatory. Making it mandatory would force the user to think about it, and consciously choose a path. We define / or . as "the root path".

If we do make it mandatory, we can introduce some logic in Kratix that would automatically patch any pre-existing Destination with spec.path = destination name if the path is not set; this would make this change a non-breaking change.

Note

Although we can make this a not-breaking-change for existing, live Kratix, all scripts generating Destinations on a brand-new Kratix may need to be updated so Destinations include a path.

I believe that making it mandatory makes the most sense. It's easy to migrate, it does not represent extra cognitive load (users already have to reason about the path, even if they leave it as empty).

Summary

• Make spec.path mandatory
• Write migration logic into the kratix controller that patch any existing destination with a path
• Update scripts and examples everywhere

[catmo-syntasso]

Making it mandatory is reasonable. deffo would want:

1. follow up to add the validation if we have conflicting paths in the spec.path
2. Examples that someone would easily copy with comments both in the example destination files and in our docs to explain how a used would configure this.

[kirederik]

TODO

• Write the code with test
• Update destination helm-charts with some default in the path
• Update examples
• Update docs
• Update demos
• Add validation webhook to ensure there are no conflicting paths

[abangser]

Only a couple of small questions

1. Does this makes the file path field redundant and if so will we be removing it?
2. A nice to have enhancement is to be able to construct the full file path from variables we can interpolate. So being and to nest by things that include the destination name but do so via variables.

Thanks for the clear write-up and party forward @kirederik !

[kirederik]

Does this makes the file path field redundant and if so will we be removing it?

No, it's still needed to determine if kratix will be writing in subfolders within the path or not. We could look into simplifying it though, maybe merging... but will leave it as a subsequent improvement

A nice to have enhancement is to be able to construct the full file path from variables we can interpolate. So being and to nest by things that include the destination name but do so via variables.

That's for what is currently being controlled by the filepath.mode right? If so, agree!