Remove APIClient dependency on the Settings model

[sumukhballal]

Issue number:

Closes

Description of changes:

We removed the APIClient's dependency on the Settings model. We did this for two dependent subcommands, set and apply.

1. For the apply command, client-side model verification before serialization was removed.
2. For the 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.

1. Single Set (Map type) works as is.

[ssm-user@control]$ apiclient set motd=test
[ssm-user@control]$ apiclient get settings.motd
{
  "settings": {
    "motd": "test"
  }
}

4. Multiple Set (Map type) works as is.

[ssm-user@control]$ apiclient set motd=random kernel.lockdown=integrity
[ssm-user@control]$ apiclient get settings.kernel.lockdown
{
  "settings": {
    "kernel": {
      "lockdown": "integrity"
    }
  }
}
[ssm-user@control]$ apiclient get settings.motd
{
  "settings": {
    "motd": "random"
  }
}

5. Invalid Set (Map type).

[ssm-user@control]$  apiclient set motd=random kernel.lockdown=random

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'

6. Passing json and key value pairs should not work.

[ssm-user@control]$ apiclient set kernel.lockdown=integrity --json '{"motd": "test"}'

Cannot specify key=value pairs and --json settings with 'set'

7. JSON form, with multiple values, internal dots, and quotes/types working as expected:

[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"
      }
    }
  }
}

8. 'Apply' works as is.

[ssm-user@control]$ apiclient apply
[settings]
motd = "from stdin, TOML"

[ssm-user@control]$ apiclient get settings.motd
{
  "settings": {
    "motd": "from stdin, TOML"
  }
}

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

[sumukhballal]

Added a clippy fix!

[bcressey]

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.

[bcressey]

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'

[sumukhballal]

Added changes to address @bcressey & @mgsharm's comments.

[bcressey]

LGTM.

In order to remove the actual model dependency from Cargo.toml, it looks like we need model::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.

[sumukhballal]

LGTM.

In order to remove the actual model dependency from Cargo.toml, it looks like we need model::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?

[bcressey]

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.

[sumukhballal]

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!

[webern]

LGTM.
In order to remove the actual model dependency from Cargo.toml, it looks like we need model::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?

So, #3949 isn't actually closed until that subsequent PR merges...