feat(config): Add `clouds add` command by dmbuil · Pull Request #1888 · gtema/openstack

dmbuil · 2026-07-26T18:14:48Z

Add osc config clouds add, a hand-written command that reads the JSON of an application credential from stdin and merges a ready-to-use cloud entry into clouds.yaml. The entry is the cloud selected with --os-cloud (required) with its credential replaced: every non-credential setting (region, TLS options, extra keys) is inherited, and of the old auth block only auth_url is kept. No authentication is performed.

The entry is built on CloudConfig (serializable since #2031) and always goes through CloudConfig::split_sensitive (#2021), so a source cloud carrying a short-lived secret (e.g. a token) is refused.

Supports --split to keep the credential secret in a separate secure.yaml (the credential id stays in clouds.yaml), --overwrite to replace an existing entry, and --file to target a specific path. Files carrying the secret are written with mode 0600 and the secret is zeroized in memory after use.

The command is dispatched in entry_point before a session is established, as it edits local files only.

Open questions

Vendor profile. The new entry is built from the cloud picked with --os-cloud after its vendor profile (profile:) has been merged in, so the vendor's settings are copied into the entry next to profile: <vendor>. That works, but later changes to the vendor profile won't reach the copied keys. Alternative: start from the cloud's own entry as written in the file (without the vendor profile merged in), so the entry keeps only profile: and picks up the vendor settings at load time. Which do you prefer?

Closes #1323

gtema

One problem that I see with this approach is that it overwrites the concrete yaml file (loosing comments and everything else in it). It would immediately break lot of things in the configs at least for me. What if instead of serde_yaml we use rust-yaml which is a "port" of python's ruamel that allows to preserve all comments of the yaml in the de/ser roundtrip to be less disruptive

gtema · 2026-07-28T13:57:25Z

+}
+
+/// One entry under the `clouds:` key of a clouds.yaml/secure.yaml file.
+#[derive(Debug, Default, Serialize)]


I guess it would be more logical to reuse structures from the sdk/core crate. This would enable adding not only app_cred connections, but also something else (who knows what) and at the same time prevent code deviation.

I still do not want us to duplicate structs. It's too much to keep in sync in addition to the fact that you need to support too many parameters here. I am thinking whether we should split the "Auth" struct of the sdk config into public and secret part so that there is some info on what belongs to clouds.yaml and what to secure.yaml. On the other side auth plugins expose information which parameter is secret and which not, so it would be possible to reconstruct it from there.

Mmmm okay,... I guess that we could drop CloudEntry/AuthBlock and build the entry from CloudConfig/Auth directly: cloning the source cloud so all settings including options carry over, and replacing only auth/auth_type.
For the clouds/secure split I'd go with the plugin metadata rather than splitting Auth: This is, requirements() already flags secrets via format: password/writeOnly and gather_auth_data relies on that, so it stays the single source of truth without an API break. That needs CloudConfig to derive Serialize and skip_serializing_if on the optional fields of Auth/CloudConfig in sdk-core though.
Side effect? Yup: --split only the secret moves to secure.yaml; application_credential_id stays in clouds.yaml. Not a fan... But less changes than full Auth rework.

In #2021 I have added CloudConfig:split_sensitive which should take care about that. Please try that one and report/adapt when it doesn't work properly

dmbuil · 2026-07-28T22:24:07Z

One problem that I see with this approach is that it overwrites the concrete yaml file (loosing comments and everything else in it). It would immediately break lot of things in the configs at least for me. What if instead of serde_yaml we use rust-yaml which is a "port" of python's ruamel that allows to preserve all comments of the yaml in the de/ser roundtrip to be less disruptive

You're right about the file rewrite, and it can get worse if one uses advanced YAML features. Round-tripping through serde_yaml also expands anchors and destroys merge keys:

 # input                        # output
 # my clouds                    clouds:
 clouds:                          base:
   base: &base                      region_name: RegionOne
     region_name: RegionOne       devstack:
   devstack:                        <<:
     <<: *base                        region_name: RegionOne
     auth:                          auth:
       auth_url: 'https://…'          auth_url: https://…

(I already knew this issue from other projects such as lazy-ssh, but thought of it as a necessary evil)

So for anyone using &base / <<: *base across regions the file comes back structurally different, not just stripped of comments.

I did try rust-yaml before replying, and I don't think it gets us there. On this input:

 # Top of file comment
 # second line
 clouds:
   # this is my devstack
   devstack:
     auth_type: password       # inline comment

load_str_with_comments -> dump_str_with_comments (with preserve_comments: true, LoaderType::RoundTrip) reorders the two header comments, drops the inline comment entirely, moves # this is my devstack to the end of the file, and emits the header comments three more times after the document. It expands anchors the same way serde_yaml does.
To be sure it wasn't my poor reading skills, I ran the crate's own examples/comment_preservation_demo.rs unmodified: on its 4-line sample it duplicates every comment 3-4 times and appends them all after the document body (o_0). The README's "✅ Full Round-trip Comment Preservation" doesn't survive a real use case; it's a young crate (1.0 in May) and that feature may need more time in the oven.

Giving it a second thought, I'd suggest instead is to stop re-serializing the file at all: parse only to validate and to detect a name collision, then splice the rendered entry into the existing text and write the original bytes back untouched everywhere else: not as elegant, but at least should work.
Appending under clouds becomes byte-exact: anchors, quoting, blanklines... all should survive because we never round-trip them. --overwrite needs locating the existing entry's line range and replace it. Anything a line-oriented edit can't do safely (flow-style clouds: {…}, multi-document files) will error out with a clear message instead of silently rewriting.

On the other hand, serde_yaml is archived, and maybe worth revisiting... but I'd keep that separate from this, for now.

Happy to go implement the splice approach unless you'd rather take this somewhere else :)

gtema · 2026-07-29T09:35:15Z

what about yamlpatch or yaml-edit?

dmbuil · 2026-07-30T09:16:46Z

I thought of the harcoded serde_yaml not to include many other "foreign crates". But now this door has been opened, I've gone through a couple of them (using a little bit of AI for the comparison).

Crate	Parses fixture	Preserves	Correct nesting	MSRV ≤1.89
noyalib	✅	✅ byte-exact	✅	✅ 1.86
yamlpatch	✅	✅ byte-exact	✅ (with workaround)	❌ 1.97
granit-parser	✅	n/a — parser only	n/a	✅ 1.81
yaml-edit	✅	❌ drops comments	❌ invalid YAML	✅ 1.70
rust-yaml	✅	❌ scrambles/dupes	—	✅ 1.85
yaml_lib	❌	❌ drops all	❌ corrupts anchors	✅ 1.88

Biggest downside of yamlpatch (the most popular) is that it imposes Rust min vers 1.97, which is way higher than OSC's 1.89. Next candidate would be noyalib, but it is a single-maintainer crate.
granit-parser would fit as well, but requires more careful implementation.

The other projects will scramble, drop or duplicate comments / advanced YAML configs.

Given that config.yaml is pretty important as to use any crate around, I wanted your guidance here, @gtema .

gtema · 2026-08-05T09:07:24Z

with good reasons I see no problem not raising msrv. I tried to be conservative with it, but a new feature can be worth it. Especially that we are now talking about the cli and there are no downstream consumers of it which is different for the SDK. Even if that ever becomes a problem we could move it into the separate crate. So let's try to use the yamlpatch

Introduces cli/config with a yaml_edit module that splices mapping entries into an existing YAML document via yamlpatch instead of round-tripping through a generic serializer, so comments and &anchor/<<: *anchor merge keys in clouds.yaml/secure.yaml survive an edit byte-for-byte. This is prep work for PR #1888's `osc config clouds add`, whose serde_yaml-based approach expands anchors and drops comments; the crate/module land separately so that PR's eventual rewire stays a small, attributable diff on top of this. yamlpatch/yamlpath require a newer Rust toolchain than the rest of the workspace, so cli/config pins its own rust-version rather than raising the workspace floor; nothing else depends on this crate, so SDK-only consumers are unaffected. CI's test job toolchain is bumped to match, since cli/* is in the default build set. Assisted-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Artem Goncharov <artem.goncharov@gmail.com>

gtema · 2026-09-17T17:31:13Z

#2004 landed the yamlpatch implementation. You can rebase now and complete your PR

dmbuil · 2026-09-17T21:39:10Z

Sweet! Will look into that, thank you very much! Sorry I didn't say a word on this before: I've been helping on yamlpatch's limitations as well as noyalib's, so that any implementation OSC would use, could be robust enough.

Add `osc config clouds add`, a hand-written command that reads the JSON of an application credential from stdin and merges a ready-to-use cloud entry into clouds.yaml. Connection settings (auth_url, region, TLS options) are inherited from the cloud selected with --os-cloud; no authentication is performed. Supports --split to keep the credential in a separate secure.yaml, --overwrite to replace an existing entry, and --file to target a specific path. Files carrying the secret are written with mode 0600 and the credential is zeroized in memory after use. `osc config` is dispatched before entry_point resolves a cloud, since most config commands need none. This one does, so it repeats the lookup itself and requires --os-cloud (or --cloud-config-from-env) rather than falling back to the interactive picker: writing a config file should not prompt-and-guess which cloud to copy. Closes gtema#1323 Signed-off-by: dmbuil <dariomartinbuil@outlook.com>

Rewire `clouds add` off serde_yaml onto the yaml_edit module, so an existing clouds.yaml or secure.yaml is edited in place: comments, &anchor/<<: *anchor merge keys and formatting outside the target entry survive byte-for-byte. --overwrite now merges into the existing entry via a new YamlDocument::merge_mapping_entry, which rewrites only scalar leaves and so keeps every comment and the entry's position. Nested mappings such as `auth` are still replaced wholesale, so a credential never half-merges; other hand-set keys on the entry are kept. Files that are empty or comment-only are handled by emitting the block directly, keeping any header comment. Tests move to yaml_serde. Signed-off-by: dmbuil <dariomartinbuil@outlook.com>

Drop the local CloudEntry/AuthBlock structs now that CloudConfig is serializable (gtema#2031). The new entry is the source cloud with its credential replaced by the application credential, so every non-credential setting is inherited; of the old auth block only auth_url is kept. The entry is always passed through CloudConfig::split_sensitive: with --split its two halves are written, without it the call still refuses a source carrying a short-lived secret such as a token. As a result --split now moves only the credential secret to secure.yaml; the id stays in clouds.yaml. Signed-off-by: dmbuil <dariomartinbuil@outlook.com>

gtema

Thanks a lot. I am pretty sure those are the last findings in the file handling logic

gtema · 2026-10-19T07:37:34Z

+    file.write_all(content.as_bytes())
+        .wrap_err_with(|| format!("cannot write {}", path.display()))?;
+    // The mode above only applies on creation; harden pre-existing files too.
+    #[cfg(unix)]


you should move that block to before you write the file to prevent the file being temporary accessible after secrets are written.

gtema · 2026-10-19T07:39:32Z

+            .wrap_err_with(|| format!("cannot create {}", parent.display()))?;
+    }
+    let mut options = std::fs::OpenOptions::new();
+    options.write(true).create(true).truncate(true);


use of truncate is not atomic. That means that once the file is opened, but write fails (e.g., disk full) the whole content may be gone. The better way to actually write files would be to write new temp files and rename them over the target after successful completion

gtema · 2026-10-19T07:50:52Z

+        // Render everything before writing anything: a same-name collision
+        // or malformed target aborts with no file touched. The rendered
+        // contents carry the credential; wipe them on drop.
+        let clouds_content = Zeroizing::new(render_target(


there is a case possible, that the entry is present completely in the secure.yaml without anything in clouds.yaml, so the same-name collision will not identify it. In the combination with split: false code result in 2 conflicting entries in clouds.yaml and secure.yaml and after overwrite eventually old credentials from secure.yaml will win

gtema · 2026-10-19T07:59:34Z

+        // `merge_mapping_entry`'s doc comment for the trade-off - keys of
+        // the old entry that this one does not set are kept, which for a
+        // credential rotation is what the user wants.
+        doc.merge_mapping_entry("clouds", cloud_name, entry)


in the build_entries you set auth_method to None, but in the combination with skip_serializing_if = None the merge_mapping_entry will not remove it from the resulting entry. It does apply only for the existing entry though.

gtema · 2026-10-19T08:04:14Z

+    // keep whatever was there as a prefix, so a comment-only file keeps its
+    // header instead of tripping the parser.
+    let Some(current) = existing.filter(|c| has_yaml_content(c)) else {
+        let mut block = format!("clouds:\n  {cloud_name}:\n");


we need to have some sanity for the cloud_name, it can break the yaml format. I would suggest to do it above in the initial cli entrypoint

Address review of the file handling in `clouds add`: - Write each target to a 0600 temporary sibling and rename it into place, so a failed write never truncates the original and a secret is never readable by others, even briefly. Both files are staged before either is moved into place. - Restrict `--cloud-name` to ASCII letters, digits, `.`, `_` and `-` so it cannot break the YAML. - On `--overwrite`, drop the old entry's `auth_methods`: an unset field is not serialized, so merging kept the stale value. - Always check secure.yaml for a same-name entry. Without `--split` its credential keys are removed on overwrite, as secure.yaml is merged over clouds.yaml and the old credential would win; with `--split` a stale `auth_type`/`auth_methods` there is dropped. Also fix `merge_mapping_entry` failing when every key of a nested mapping is replaced (e.g. a password rotated to an application credential in secure.yaml): stale keys are now removed after the new ones are inserted, since emptying a mapping makes yamlpatch remove its ancestors too. Signed-off-by: dmbuil <dariomartinbuil@outlook.com>

dmbuil mentioned this pull request Jul 26, 2026

[Feature Request] output application credentials as clouds.yaml #1323

Open

gtema reviewed Jul 28, 2026

View reviewed changes

tobiaskohlbau mentioned this pull request Aug 6, 2026

support listing available clouds via cli #1916

Open

gtema mentioned this pull request Sep 17, 2026

feat(config): Add YAML edit foundation module #2004

Merged

dmbuil force-pushed the feature-config-clouds-add branch 2 times, most recently from d562815 to b05d815 Compare September 19, 2026 23:19

dmbuil mentioned this pull request Oct 12, 2026

feat(sdk): Make CloudConfig serializable #2031

Merged

dmbuil force-pushed the feature-config-clouds-add branch from b05d815 to 1992294 Compare October 13, 2026 19:21

dmbuil added 3 commits October 16, 2026 22:34

dmbuil force-pushed the feature-config-clouds-add branch from 1992294 to f05f777 Compare October 16, 2026 20:43

gtema requested changes Oct 19, 2026

View reviewed changes

Conversation

dmbuil commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Open questions

Uh oh!

gtema left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmbuil commented Jul 28, 2026

Uh oh!

gtema commented Jul 29, 2026

Uh oh!

dmbuil commented Jul 30, 2026

Uh oh!

gtema commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gtema commented Sep 17, 2026

Uh oh!

dmbuil commented Sep 17, 2026

Uh oh!

gtema left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dmbuil commented Jul 26, 2026 •

edited

Loading

gtema commented Aug 5, 2026 •

edited

Loading