Juju 2.9.31 breaks yaml format accepted by `juju add-credential`

Bug #1976620 reported by Simon Fels
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Canonical Juju
Fix Released
Critical
Thomas Miller

Bug Description

With Juju < 2.9.31 our automated scripts register credentials for a cloud via `juju add-credential`with a file in the following format:

$ cat creds.yaml
credentials:
  appliance:
    admin:
      auth-type: certificate
      server-cert: |
        -----BEGIN CERTIFICATE-----
        ...
        -----END CERTIFICATE-----
      client-cert: |
        -----BEGIN CERTIFICATE-----
        ...
        -----END CERTIFICATE-----
      client-key: |
        -----BEGIN PRIVATE KEY-----
        ...
        -----END PRIVATE KEY-----

After https://github.com/juju/juju/pull/14040/files got merged a

$ juju add-credentials creds.yaml

now receives the following error:

23:26:33 + sudo -u ubuntu -H /snap/bin/juju add-credential --client appliance -f /var/snap/anbox-cloud-appliance/x1/.tmp.2yXDhg/creds.yaml
23:26:33 + tee -a /var/snap/anbox-cloud-appliance/x1/.tmp.2yXDhg/juju.log
23:26:33 ERROR expanding file paths for credential: determining file path value for credential attribute: invalid file path: stat /var/snap/anbox-cloud-appliance/x1/-----BEGIN CERTIFICATE-----
....

This breaks any user using the Anbox Cloud Appliance taking the latest Juju from the 2.9 track as a base. A quick fix for this would be much appreciated!

Changed in juju:
assignee: nobody → Thomas Miller (tlmiller)
importance: Undecided → Critical
milestone: none → 2.9.32
status: New → Triaged
Revision history for this message
John A Meinel (jameinel) wrote :

Ultimately inside the database, we store the expanded form as described above (with the content of the certificate as the value for the 'server-cert' key)
In code, we have logic that says "if this is a filename, expand it to the contents" so that people can have a creds.yaml that points to an external file.
It seems that prior to 14040 we would fall back to just the content if we couldn't expand it as a path, and Thomas's change causes us to say "if it isn't a path, fail entirely".
Where we have done this sort of thing in other config, is that we would have 2 distinct keys. one for 'server-cert-path' and one for 'server-cert'. And if you had 'server-cert-path' set, it would read the contents of that path, and set it into 'server-cert'.
I don't know why we went down the route of having the user supply a path in the same key as the actual content (you end up with this sort of confusion between whether the value is the content or a reference to the content.)

A short term workaround is to change it with paths to certificates which should work in old and new versions of juju, and we can look to restore the ambiguous behavior for 2.9.32, and possibly provide 'server-cert-path' as a way to have unambiguous behavior in the future.

Revision history for this message
Simon Fels (morphis) wrote :

A short term workaround is tricky as it requires us to do a completely new release where we just did one last Wednesday, a day before Juju 2.9.31 came out :-)

The Anbox Cloud Appliance sets up a LXD based Juju controller fully automatically and is shipped through cloud marketplaces to customers. So as the juju snap is refreshed and users start setting things up, it will break immediately.

However, let me see that we land a fix and push it ASAP after validation passes.

Revision history for this message
Thomas Miller (tlmiller) wrote :

Hey Simon,

The way we describe credentials in Juju says that these keys should be filenames. Previously this expansion or checking wasn't occurring and part of my PR made sure this check was happening as more validation had to happen on the server certificate and the Juju code base expects the value to be the expanded value. All of the examples and code we had all referred to the keys being file names so we assumed wrongly that it would be safe to force the check.

I will get a fix up now to accept both values. For Juju 3.0 I would like to find a better way to do this so we aren't giving double meaning to the keys.

tlm

Changed in juju:
status: Triaged → In Progress
Revision history for this message
Simon Fels (morphis) wrote :

I agreed the format was never well documented but there are other people using it the same way (e.g. https://discourse.charmhub.io/t/adding-a-lxd-remote-messing-with-credentials/4700, https://discourse.charmhub.io/t/setting-up-lxd-cluster-as-a-juju-cloud/73/2 and I know OSM has some documentation covering this too).

Revision history for this message
Thomas Miller (tlmiller) wrote :
Changed in juju:
status: In Progress → Fix Committed
Revision history for this message
Ian Booth (wallyworld) wrote :

As a work around until 2.9.32 is released, you can snap install juju 2.9/edge
If you bootstrap with it, just use --agent-version to get 2.9.31 controllers, eg

/snap/bin/juju bootstrap lxd --agent-version=2.9.31

Revision history for this message
Simon Fels (morphis) wrote :

@Ian We sadly cannot workaround as this affects users in the field. Juju is entirely hidden from them and influencing the process here is not possible.

We're releasing a version today which switches to file paths instead of textual values in the YAML

Changed in juju:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.