Closed sumukhballal closed 5 months ago
Added a clippy fix!
When we're in "CLI mode" (not used as a library), can we clean up this error?
Failed to change settings: Failed PATCH request to '/settings/keypair?tx=apiclient-set-tTZRnB3Bi2B8ksZp': Status 400 when PATCHing /settings/keypair?tx=apiclient-set-tTZRnB3Bi2B8ksZp: Unable to match your input to the data model. We may not have enough type information. Please try the --json input form. Cause: Error deserializing scalar value: Unable to deserialize into Lockdown: Invalid Linux lockdown mode 'random'
# can be just this:
Unable to match your input to the data model. We may not have enough type information. Please try the --json input form. Cause: Error deserializing scalar value: Unable to deserialize into Lockdown: Invalid Linux lockdown mode 'random'
In general, I'd like to keep the user-facing output the same before and after this change. Scripts and what not can take dependencies on specific output messages.
Same for apiclient apply
- we should get the cleaned-up errors like this:
$ apiclient apply <<EOF
> [settings.kernel]
> lockdown = "random"
> EOF
Failed to apply settings: Failed to deserialize settings from '-' into this variant's model: Unable to deserialize into Lockdown: Invalid Linux lockdown mode 'random'
Added changes to address @bcressey & @mgsharm's comments.
LGTM.
In order to remove the actual model dependency from
Cargo.toml
, it looks like we needmodel::exec
to move somewhere else. Are you planning to tackle that next?It's not strictly required here but it would be easy to make the mistake of using more of the model than that, including
Settings
which would be broken.
@bcressey Yeah, Ill add that as part of another PR. I was wondering if it would be a good idea to add those types to the 'modeled-types' package & update any of its references. Like here: https://github.com/bottlerocket-os/bottlerocket/blob/develop/sources/models/modeled-types/src/shared.rs
or just add them into APIClient?
I was wondering if it would be a good idea to add those types to the 'modeled-types' package & update any of its references.
Maybe a shared api-types crate that is just for apiserver & apiclient alignment? modeled-types is somewhat bound up in settings concerns currently, and I foresee it moving to the settings SDK for ease of out-of-tree use.
I was wondering if it would be a good idea to add those types to the 'modeled-types' package & update any of its references.
Maybe a shared api-types crate that is just for apiserver & apiclient alignment? modeled-types is somewhat bound up in settings concerns currently, and I foresee it moving to the settings SDK for ease of out-of-tree use.
Sounds good. Will raise a PR shortly!
LGTM. In order to remove the actual model dependency from
Cargo.toml
, it looks like we needmodel::exec
to move somewhere else. Are you planning to tackle that next? It's not strictly required here but it would be easy to make the mistake of using more of the model than that, includingSettings
which would be broken.@bcressey Yeah, Ill add that as part of another PR. I was wondering if it would be a good idea to add those types to the 'modeled-types' package & update any of its references. Like here: https://github.com/bottlerocket-os/bottlerocket/blob/develop/sources/models/modeled-types/src/shared.rs
or just add them into APIClient?
So, #3949 isn't actually closed until that subsequent PR merges...
Issue number:
Closes
Description of changes:
We removed the APIClient's dependency on the Settings model. We did this for two dependent subcommands,
set
andapply
.apply
command, client-side model verification before serialization was removed.set
command, for key-value pair inputs a new APIserver endpoint /settings/keypair was introduced. This had its benefits as it allowed the current JSON inputs to work as is, but allowed the transfer of key-pair specific logic from the Client to the Server without any major changes i.e no real change in the data-format for the APIServer to be input (key-value or json) aware.Testing done:
Testing was done
locally
& on an aws-ecs-1 AMI instance. Additionally, Testing between existing AMI and new AMI;s with the code changes was done to prove consistent behavior.Failed to change settings: Failed PATCH request to '/settings/keypair?tx=apiclient-set-tTZRnB3Bi2B8ksZp': Status 400 when PATCHing /settings/keypair?tx=apiclient-set-tTZRnB3Bi2B8ksZp: Unable to match your input to the data model. We may not have enough type information. Please try the --json input form. Cause: Error deserializing scalar value: Unable to deserialize into Lockdown: Invalid Linux lockdown mode 'random'
[ssm-user@control]$ apiclient set kernel.lockdown=integrity --json '{"motd": "test"}'
Cannot specify key=value pairs and --json settings with 'set'
[ssm-user@control]$ apiclient set --json '{"kernel": {"sysctl": {"vm.max_map_count": "888888", "user.max_user_namespaces": "9999"}}}' [ssm-user@control]$ [ssm-user@control]$ apiclient get settings.kernel { "settings": { "kernel": { "lockdown": "integrity", "sysctl": { "user.max_user_namespaces": "9999", "vm.max_map_count": "888888" } } } }
[ssm-user@control]$ apiclient apply [settings] motd = "from stdin, TOML"
[ssm-user@control]$ apiclient get settings.motd { "settings": { "motd": "from stdin, TOML" } }