fix: warn about unknown manifest fields

[zhhuabj]

Warn when unrecognized fields occur in fixed manifest schemas so misspelled or misplaced configuration is not silently ignored.

Ignore unknown fields and feature names after warning while preserving validation failures for invalid known values. Keep intentional charm and storage extension points unchanged, and avoid mutating manifest input while parsing.

Closes-Bug: #2162969
Jira card: https://warthogs.atlassian.net/browse/SI-4781

[zhhuabj]

##Self-test result

#Test 1: Top level and nested unknown fields
cat > "$HOME/manifest-warning-and-error.yaml" <<'EOF'
preseed:
user:
remote_access_location: remote
core:
config:
user:
remote_access_locaion: remote
external-network:
network_type: invalid
EOF
ubuntu@sunbeam:~$ sunbeam configure --manifest "$HOME/manifest-warning-and-error.yaml"
Unknown manifest key: preseed
Unknown manifest key: remote_access_locaion
...

#Test 2: Error alias
cat > "$HOME/manifest-alias-warning.yaml" <<'EOF'
core:
config:
k8s_addons:
loadbalancer: metallb
external-network:
network_type: invalid
EOF
ubuntu@sunbeam:~$ sunbeam configure --manifest "$HOME/manifest-alias-warning.yaml"
Unknown manifest key: k8s_addons
...

#Test 3: Unknown Features
cat > "$HOME/manifest-feature-warning.yaml" <<'EOF'
features:
loadbalncer: {}
loadbalancer:
config:
lb_mgmt_secgroup_ids: invalid
EOF
ubuntu@sunbeam:~$ sunbeam configure --manifest "$HOME/manifest-feature-warning.yaml"
Feature loadbalncer is not found in feature manager
...

[hmlanigan]

Please add the jira card to the PR description per the template.

[hmlanigan]

Given how hard clean up can be, and after discussions with the team. These warnings should be failures instead. Listing all known bad manifest data to start.

[zhhuabj]

Please add the jira card to the PR description per the template.

done, add 'Jira card: https://warthogs.atlassian.net/browse/SI-4781' now

[zhhuabj]

**hmlanigan **

Hi @hmlanigan , agreed, and changed. Unknown manifest data is now failured instead of warned, here is my self-test result - https://pastebin.com/hHjv7jmU

[gboutry]

Thanks for this change, it's overall in great shape.

Except the part of loading the manifest, I'm not sure the new addition is actually helpful, if you care to comment why, please.

[gboutry]

I'm really not a fan of

try:
  ...
else:
  ...

Does this addition help the code make more sense?

[zhhuabj]

Hi @gboutry , thanks for the review. This else is needed, see the following code:

try:
client = self.get_client()
manifest_data = yaml.safe_load(
client.cluster.get_latest_manifest()["data"]
)
except ClusterServiceUnavailableException:
...
except ConfigItemNotFoundException:
...
except ValueError:
LOG.debug("Failed to get clusterd client, might no be bootstrapped, ...")
else:
override_manifest = self.parse_manifest(manifest_data)
LOG.debug("Manifest loaded from clusterd")

Because parse_manifest() now raises ValueError for
unknown feature names, while the existing except ValueError is there to catch
get_client() failing (e.g. MAAS raising Clusterd address not set.).

If the parse stayed inside the try, a ValueError from an unknown feature in
the clusterd manifest would be swallowed by that handler and the code would
silently fall back to the embedded manifest — exactly the "silently ignored"
behaviour this PR removes.

So try covers "get a client and read the manifest", else covers "parse it".