Skip to content

cloud-id: publish /run/cloud-init/cloud-id-<cloud-type> files #1244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TheRealFalcon merged 3 commits into from
Feb 10, 2022
Merged

cloud-id: publish /run/cloud-init/cloud-id-<cloud-type> files #1244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c63e80
cloud-id: publish /run/cloud-init/cloud-id-<cloud-type> files
blackboxsw Feb 4, 2022
5731a01
tests: assert symlink for /run/cloud-init/cloud-id to specific cloud-id
blackboxsw Feb 10, 2022
13ec578
cloud-id: include trailing newline in /run/cloud-init/cloud-id
blackboxsw Feb 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions cloudinit/sources/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,9 @@ def _get_standardized_metadata(self, instance_data):
"_beta_keys": ["subplatform"],
"availability-zone": availability_zone,
"availability_zone": availability_zone,
"cloud_id": canonical_cloud_id(
self.cloud_name, self.region, self.platform_type
),
"cloud-name": self.cloud_name,
"cloud_name": self.cloud_name,
"distro": sysinfo["dist"][0],
Expand Down Expand Up @@ -408,6 +411,16 @@ def persist_instance_data(self):
json_sensitive_file = os.path.join(
self.paths.run_dir, INSTANCE_JSON_SENSITIVE_FILE
)
cloud_id = instance_data["v1"].get("cloud_id", "none")
cloud_id_file = os.path.join(self.paths.run_dir, "cloud-id")
util.write_file(f"{cloud_id_file}-{cloud_id}", f"{cloud_id}\n")
if os.path.exists(cloud_id_file):
prev_cloud_id_file = os.path.realpath(cloud_id_file)
else:
prev_cloud_id_file = cloud_id_file
util.sym_link(f"{cloud_id_file}-{cloud_id}", cloud_id_file, force=True)
if prev_cloud_id_file != cloud_id_file:
util.del_file(prev_cloud_id_file)
Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon Feb 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? /run/cloud-init gets removed every boot. Do we support changing a datasource post-boot? It doesn't hurt anything though.

Copy link
Copy Markdown
Collaborator Author

@blackboxsw blackboxsw Feb 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know of some AWS customers whose upgrade and deployment strategy involves executing cloud-init clean --logs; cloud-init init --locl; cloud-init init. If someone injected NoCloud seed files before upgrade I could see a stale /run/cloud-init/cloud-id-ec2 left around instead of /run/cloud-init/cloud-id-no-cloud. Just didn't want occasion to have artifactts around that are no longer correct. There also may be folks that live-migrate a VM to a sandboxed environment which may no longer have visibility to the original IMDS. If they trigger a persist_instance_data() we'd want to cleanup any no longer applicable DS in case something else reacts to it's presence.

write_json(json_sensitive_file, processed_data, mode=0o600)
json_file = os.path.join(self.paths.run_dir, INSTANCE_JSON_FILE)
# World readable
Expand Down
26 changes: 26 additions & 0 deletions tests/integration_tests/modules/test_combined.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,16 @@ def test_correct_datasource_detected(
== parsed_datasource
)

def test_cloud_id_file_symlink(self, class_client: IntegrationInstance):
cloud_id = class_client.execute("cloud-id").stdout
expected_link_output = (
"'/run/cloud-init/cloud-id' -> "
f"'/run/cloud-init/cloud-id-{cloud_id}'"
)
assert expected_link_output == str(
class_client.execute("stat -c %N /run/cloud-init/cloud-id")
)

def _check_common_metadata(self, data):
assert data["base64_encoded_keys"] == []
assert data["merged_cfg"] == "redacted for non-root user"
Expand Down Expand Up @@ -249,6 +259,10 @@ def test_instance_json_lxd(self, class_client: IntegrationInstance):
v1_data = data["v1"]
assert v1_data["cloud_name"] == "unknown"
assert v1_data["platform"] == "lxd"
assert v1_data["cloud_id"] == "lxd"
assert f"{v1_data['cloud_id']}" == client.read_from_file(
"/run/cloud-init/cloud-id-lxd"
)
assert (
v1_data["subplatform"]
== "seed-dir (/var/lib/cloud/seed/nocloud-net)"
Expand All @@ -270,6 +284,10 @@ def test_instance_json_lxd_vm(self, class_client: IntegrationInstance):
v1_data = data["v1"]
assert v1_data["cloud_name"] == "unknown"
assert v1_data["platform"] == "lxd"
assert v1_data["cloud_id"] == "lxd"
assert f"{v1_data['cloud_id']}" == client.read_from_file(
"/run/cloud-init/cloud-id-lxd"
)
assert any(
[
"/var/lib/cloud/seed/nocloud-net" in v1_data["subplatform"],
Expand All @@ -291,6 +309,11 @@ def test_instance_json_ec2(self, class_client: IntegrationInstance):
v1_data = data["v1"]
assert v1_data["cloud_name"] == "aws"
assert v1_data["platform"] == "ec2"
# Different regions will show up as ec2-(gov|china)
assert v1_data["cloud_id"].startswith("ec2")
assert f"{v1_data['cloud_id']}" == client.read_from_file(
"/run/cloud-init/cloud-id-ec2"
)
assert v1_data["subplatform"].startswith("metadata")
assert (
v1_data["availability_zone"] == client.instance.availability_zone
Expand All @@ -310,6 +333,9 @@ def test_instance_json_gce(self, class_client: IntegrationInstance):
v1_data = data["v1"]
assert v1_data["cloud_name"] == "gce"
assert v1_data["platform"] == "gce"
assert f"{v1_data['cloud_id']}" == client.read_from_file(
"/run/cloud-init/cloud-id-gce"
)
assert v1_data["subplatform"].startswith("metadata")
assert v1_data["availability_zone"] == client.instance.zone
assert v1_data["instance_id"] == client.instance.instance_id
Expand Down
46 changes: 44 additions & 2 deletions tests/unittests/sources/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,11 @@ def test_get_data_writes_json_instance_data_on_success(self):
"dist": ["ubuntu", "20.04", "focal"],
}
with mock.patch("cloudinit.util.system_info", return_value=sys_info):
datasource.get_data()
with mock.patch(
"cloudinit.sources.canonical_cloud_id",
return_value="canonical_cloud_id",
):
datasource.get_data()
json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp)
content = util.load_file(json_file)
expected = {
Expand All @@ -390,6 +394,7 @@ def test_get_data_writes_json_instance_data_on_success(self):
"_beta_keys": ["subplatform"],
"availability-zone": "myaz",
"availability_zone": "myaz",
"cloud_id": "canonical_cloud_id",
"cloud-name": "subclasscloudname",
"cloud_name": "subclasscloudname",
"distro": "ubuntu",
Expand Down Expand Up @@ -562,7 +567,11 @@ def test_get_data_writes_json_instance_data_sensitive(self):
datasource.sensitive_metadata_keys,
)
with mock.patch("cloudinit.util.system_info", return_value=sys_info):
datasource.get_data()
with mock.patch(
"cloudinit.sources.canonical_cloud_id",
return_value="canonical-cloud-id",
):
datasource.get_data()
sensitive_json_file = self.tmp_path(INSTANCE_JSON_SENSITIVE_FILE, tmp)
content = util.load_file(sensitive_json_file)
expected = {
Expand All @@ -583,6 +592,7 @@ def test_get_data_writes_json_instance_data_sensitive(self):
"_beta_keys": ["subplatform"],
"availability-zone": "myaz",
"availability_zone": "myaz",
"cloud_id": "canonical-cloud-id",
"cloud-name": "subclasscloudname",
"cloud_name": "subclasscloudname",
"distro": "ubuntu",
Expand Down Expand Up @@ -666,6 +676,38 @@ def test_persist_instance_data_writes_ec2_metadata_when_set(self):
{"ec2stuff": "is good"}, instance_data["ds"]["ec2_metadata"]
)

def test_persist_instance_data_writes_canonical_cloud_id_and_symlink(self):
"""canonical-cloud-id class attribute is set, persist to json."""
tmp = self.tmp_dir()
datasource = DataSourceTestSubclassNet(
self.sys_cfg, self.distro, Paths({"run_dir": tmp})
)
cloud_id_link = os.path.join(tmp, "cloud-id")
cloud_id_file = os.path.join(tmp, "cloud-id-my-cloud")
cloud_id2_file = os.path.join(tmp, "cloud-id-my-cloud2")
for filename in (cloud_id_file, cloud_id_link, cloud_id2_file):
self.assertFalse(
os.path.exists(filename), "Unexpected link found {filename}"
)
with mock.patch(
"cloudinit.sources.canonical_cloud_id", return_value="my-cloud"
):
datasource.get_data()
self.assertEqual("my-cloud\n", util.load_file(cloud_id_link))
# A symlink with the generic /run/cloud-init/cloud-id link is present
self.assertTrue(util.is_link(cloud_id_link))
# When cloud-id changes, symlink and content change
with mock.patch(
"cloudinit.sources.canonical_cloud_id", return_value="my-cloud2"
):
datasource.persist_instance_data()
self.assertEqual("my-cloud2\n", util.load_file(cloud_id2_file))
# Previous cloud-id-<cloud-type> file removed
self.assertFalse(os.path.exists(cloud_id_file))
# Generic link persisted which contains canonical-cloud-id as content
self.assertTrue(util.is_link(cloud_id_link))
self.assertEqual("my-cloud2\n", util.load_file(cloud_id_link))

def test_persist_instance_data_writes_network_json_when_set(self):
"""When network_data.json class attribute is set, persist to json."""
tmp = self.tmp_dir()
Expand Down