mgr/cephadm: store contianer registry credentials in config-key #43889

Daniel-Pivonka · 2021-11-11T15:19:57Z

store credentials in a more secure place

Fixes: https://tracker.ceph.com/issues/53269
Signed-off-by: Daniel Pivonka dpivonka@redhat.com

src/cephadm/cephadm

liewegas · 2021-11-15T16:02:27Z

src/pybind/mgr/cephadm/module.py

@@ -277,6 +277,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule,
            default='*',
            desc='PlacementSpec describing on which hosts to manage /etc/ceph/ceph.conf',
        ),
+        # not used anymore


I think we should remove these options entirely. If I'm reading the code properly, the migration config rm calls should still work.

i wanted to do that but i was having trouble getting the old values during the migration.

once the new mgr takes over during an upgrade and with those removed
registry_url = self.mgr.get_module_option('registry_url')
causes
RuntimeError: Config option 'registry_url' is not in CephadmOrchestrator.MODULE_OPTIONS

and

ret, registry_url, err = self.mgr.mon_command({ 'prefix': 'config get', 'who': 'mgr', 'key': 'mgr/cephadm/registry_url', })

returns a return code of -2 and no value for registry_url

additionally trying to run

ceph config get mgr mgr/cephadm/registry_url

returns Error ENOENT:

I think I should add a --force to config get... WDYT @sebastian-philipp ?

Not yet made my mind about this one. IMO this is an extraordinary edge case. Is it worth building extra code for this?

sebastian-philipp · 2021-11-16T10:46:59Z

src/pybind/mgr/cephadm/migrations.py

+            self.mgr.set_module_option('registry_url', None)
+            self.mgr.check_mon_command({
+                'prefix': 'config rm',
+                'who': 'mgr',
+                'key': 'mgr/cephadm/registry_url',
+            })


think this is redundant. See

ceph/src/mgr/PyModule.cc

Lines 199 to 204 in 8713c24

if (val) {

jf.dump_string("prefix", "config set");

jf.dump_string("value", *val);

} else {

jf.dump_string("prefix", "config rm");

}

without this the values do not get removed from config dump

src/pybind/mgr/cephadm/serve.py

sebastian-philipp · 2021-11-16T12:36:53Z

While you're here, https://docs.ceph.com/en/latest/cephadm/install/#further-information-about-cephadm-bootstrap is bad as well. Can we deprecate cephadm bootstrap --registry-password? I.e. only document --registry-json

github-actions · 2021-11-22T11:56:38Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

sebastian-philipp · 2021-11-30T09:27:20Z

2021-11-29T16:40:23.637 DEBUG:teuthology.orchestra.run.smithi026:> sudo /home/ubuntu/cephtest/cephadm --image docker.io/ceph/ceph:v16.2.4 shell -c /etc/ceph/ceph.conf -k /etc/ceph/ceph.client.admin.keyring --fsid 65fb89d2-5130-11ec-8c2d-001a4aab830c -- bash -c 'ceph config get mgr mgr/cephadm/migration_current | grep 3'
2021-11-29T16:40:24.741 INFO:journalctl@ceph.mon.smithi026.smithi026.stdout:Nov 29 16:40:24 smithi026 ceph-mon[106998]: pgmap v356: 129 pgs: 129 active+clean; 214 MiB data, 2.7 GiB used, 712 GiB / 715 GiB avail; 255 B/s rd, 0 op/s
2021-11-29T16:40:25.037 INFO:journalctl@ceph.mon.smithi119.smithi119.stdout:Nov 29 16:40:24 smithi119 ceph-mon[74838]: pgmap v356: 129 pgs: 129 active+clean; 214 MiB data, 2.7 GiB used, 712 GiB / 715 GiB avail; 255 B/s rd, 0 op/s
2021-11-29T16:40:25.968 DEBUG:teuthology.orchestra.run:got remote process result: 1
2021-11-29T16:40:25.969 ERROR:teuthology.run_tasks:Saw exception from tasks.
Traceback (most recent call last):
  File "/home/teuthworker/src/git.ceph.com_git_teuthology_a0bc318848afe71da1fbf34e1e1b44738d8e3d6d/teuthology/run_tasks.py", line 91, in run_tasks
    manager = run_one_task(taskname, ctx=ctx, config=config)
  File "/home/teuthworker/src/git.ceph.com_git_teuthology_a0bc318848afe71da1fbf34e1e1b44738d8e3d6d/teuthology/run_tasks.py", line 70, in run_one_task
    return task(**kwargs)
  File "/home/teuthworker/src/git.ceph.com_ceph-c_caa4f4125affaece3c1649c79f1b56fa72f9867e/qa/tasks/cephadm.py", line 1101, in shell
    extra_cephadm_args=args)
  File "/home/teuthworker/src/git.ceph.com_ceph-c_caa4f4125affaece3c1649c79f1b56fa72f9867e/qa/tasks/cephadm.py", line 47, in _shell
    **kwargs
  File "/home/teuthworker/src/git.ceph.com_git_teuthology_a0bc318848afe71da1fbf34e1e1b44738d8e3d6d/teuthology/orchestra/remote.py", line 509, in run
    r = self._runner(client=self.ssh, name=self.shortname, **kwargs)
  File "/home/teuthworker/src/git.ceph.com_git_teuthology_a0bc318848afe71da1fbf34e1e1b44738d8e3d6d/teuthology/orchestra/run.py", line 455, in run
    r.wait()
  File "/home/teuthworker/src/git.ceph.com_git_teuthology_a0bc318848afe71da1fbf34e1e1b44738d8e3d6d/teuthology/orchestra/run.py", line 161, in wait
    self._raise_for_status()
  File "/home/teuthworker/src/git.ceph.com_git_teuthology_a0bc318848afe71da1fbf34e1e1b44738d8e3d6d/teuthology/orchestra/run.py", line 183, in _raise_for_status
    node=self.hostname, label=self.label
teuthology.exceptions.CommandFailedError: Command failed on smithi026 with status 1: "sudo /home/ubuntu/cephtest/cephadm --image docker.io/ceph/ceph:v16.2.4 shell -c /etc/ceph/ceph.conf -k /etc/ceph/ceph.client.admin.keyring --fsid 65fb89d2-5130-11ec-8c2d-001a4aab830c -- bash -c 'ceph config get mgr mgr/cephadm/migration_current | grep 3'"

https://pulpito.ceph.com/swagner-2021-11-29_16:02:52-orch:cephadm-wip-swagner3-testing-2021-11-29-1155-distro-default-smithi/6533675

See

ceph/qa/suites/orch/cephadm/mgr-nfs-upgrade/4-final.yaml

Line 10 in 174ad99

- ceph config get mgr mgr/cephadm/migration_current | grep 3

github-actions · 2021-12-01T17:28:21Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

Signed-off-by: Daniel Pivonka <dpivonka@redhat.com>

sebastian-philipp · 2021-12-16T10:04:33Z

https://pulpito.ceph.com/swagner-2021-12-15_14:51:15-orch:cephadm-wip-swagner-testing-2021-12-15-1026-distro-default-smithi/

Daniel-Pivonka added the cephadm label Nov 11, 2021

Daniel-Pivonka requested a review from a team as a code owner November 11, 2021 15:19

github-actions bot added the pybind label Nov 11, 2021

sebastian-philipp reviewed Nov 11, 2021

View reviewed changes

src/cephadm/cephadm Outdated Show resolved Hide resolved

Daniel-Pivonka force-pushed the cephadm-reg-cred branch 4 times, most recently from 59805e8 to 3396511 Compare November 15, 2021 14:26

liewegas reviewed Nov 15, 2021

View reviewed changes

sebastian-philipp suggested changes Nov 16, 2021

View reviewed changes

github-actions bot added the needs-rebase label Nov 22, 2021

Daniel-Pivonka force-pushed the cephadm-reg-cred branch from 3396511 to 423ff2e Compare November 22, 2021 17:34

github-actions bot added documentation and removed needs-rebase labels Nov 22, 2021

Daniel-Pivonka force-pushed the cephadm-reg-cred branch from 423ff2e to 28b961e Compare November 22, 2021 18:07

sebastian-philipp mentioned this pull request Nov 24, 2021

Move ceph-salt-registry-json creation outside bootstrap ceph/ceph-salt#467

Merged

sebastian-philipp approved these changes Nov 26, 2021

View reviewed changes

sebastian-philipp added the wip-swagner3-testing label Nov 26, 2021

sebastian-philipp removed the wip-swagner3-testing label Nov 30, 2021

github-actions bot added the needs-rebase label Dec 1, 2021

mgr/cephadm: store contianer registry credentials in config-key

2c29286

Signed-off-by: Daniel Pivonka <dpivonka@redhat.com>

Daniel-Pivonka force-pushed the cephadm-reg-cred branch from 28b961e to 2c29286 Compare December 1, 2021 17:30

github-actions bot removed the needs-rebase label Dec 1, 2021

sebastian-philipp added the wip-swagner-testing My Teuthology tests label Dec 2, 2021

sebastian-philipp merged commit e911f41 into ceph:master Dec 16, 2021

sebastian-philipp mentioned this pull request Jan 19, 2022

pacific: mgr/cephadm: store contianer registry credentials in config-key #44658

Merged

adk3798 mentioned this pull request Jun 12, 2023

mgr/cephadm: storing prometheus/alertmanager credentials in monstore #51987

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mgr/cephadm: store contianer registry credentials in config-key #43889

mgr/cephadm: store contianer registry credentials in config-key #43889

Daniel-Pivonka commented Nov 11, 2021 •

edited

liewegas Nov 15, 2021

Daniel-Pivonka Nov 15, 2021

liewegas Nov 16, 2021

sebastian-philipp Nov 16, 2021

sebastian-philipp Nov 16, 2021

Daniel-Pivonka Nov 22, 2021

sebastian-philipp commented Nov 16, 2021 •

edited

github-actions bot commented Nov 22, 2021

sebastian-philipp commented Nov 30, 2021

github-actions bot commented Dec 1, 2021

sebastian-philipp commented Dec 16, 2021

	if (val) {
	jf.dump_string("prefix", "config set");
	jf.dump_string("value", *val);
	} else {
	jf.dump_string("prefix", "config rm");
	}

mgr/cephadm: store contianer registry credentials in config-key #43889

mgr/cephadm: store contianer registry credentials in config-key #43889

Conversation

Daniel-Pivonka commented Nov 11, 2021 • edited

liewegas Nov 15, 2021

Choose a reason for hiding this comment

Daniel-Pivonka Nov 15, 2021

Choose a reason for hiding this comment

liewegas Nov 16, 2021

Choose a reason for hiding this comment

sebastian-philipp Nov 16, 2021

Choose a reason for hiding this comment

sebastian-philipp Nov 16, 2021

Choose a reason for hiding this comment

Daniel-Pivonka Nov 22, 2021

Choose a reason for hiding this comment

sebastian-philipp commented Nov 16, 2021 • edited

github-actions bot commented Nov 22, 2021

sebastian-philipp commented Nov 30, 2021

github-actions bot commented Dec 1, 2021

sebastian-philipp commented Dec 16, 2021

Daniel-Pivonka commented Nov 11, 2021 •

edited

sebastian-philipp commented Nov 16, 2021 •

edited