Skip to content
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.

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

[Change Proposal] Add support for required conditional groups of variables #744

Open
jsoriano opened this issue May 3, 2024 · 11 comments · May be fixed by #855
Open

[Change Proposal] Add support for required conditional groups of variables #744

jsoriano opened this issue May 3, 2024 · 11 comments · May be fixed by #855
Assignees
Labels
discuss Issue needs discussion
Milestone

Comments

@jsoriano
Copy link
Member

jsoriano commented May 3, 2024

There are cases where there are sets of variables that are optional, but it is required to set at least one of them. This is frequent for example when there are different available authentication options.

For example in Cloudflare there are three variables defined as optional: auth_token, auth_email and auth_key. Either the token needs to be set, or the email and the key.

We need a way to express this in the package manifests.

There are some options to explore:

Option 1: Variable groups

So it is possible to define "groups" in a similar fashion to select, and the selected group can have its own restrictions, something like this:

      - name: auth
        type: select_vargroup
        required: true
        options:
          - name: key
            title: Authentication using email and password
            vars:
              - name: auth_email
                type: text
                title: Auth Email
                description: The Auth Email. Needs to be used with an Auth Key. Do not fill if you are using an Auth Token.
                multi: false
                required: true
                show_user: true
              - name: auth_key
                type: password
                title: Auth Key
                description: The Auth Key. Needs to be used with an Auth Email. Do not fill if you are using an Auth Token.
                multi: false
                required: true
                show_user: true
                secret: true
          - name: token
            title: Authentication using token
            vars:
              - name: auth_token
                type: password
                title: Auth token
                description: The auth token. If set, Auth Email and Auth Key will be ignored.
                required: true
                multi: false
                show_user: true
                secret: true

This would also require changes in Fleet UI.

Option 2: Groups of requirements

Define a list of groups that must be configured. Something like this would mean what is wanted in the Cloudflare integration:

required_vars:
  - [auth_email, auth_key]
  - [auth_token]

Understood as (auth_email AND auth_key) OR (auth_token).

This option wouldn't require breaking changes in packages. It will also require changes in Fleet UI.

@jsoriano jsoriano added the discuss Issue needs discussion label May 3, 2024
@criamico
Copy link
Contributor

criamico commented May 6, 2024

Kibana related PR: elastic/kibana#182645

@seanrathier
Copy link
Contributor

The proposal presented would help out with the CSPM integration. The problem statement aligns with the CSPM challenge where we need to have all the stream variables optional because we have the optionsaccess_key and secret_key need to be required together but session_token should be required by itself. I think this case looks like option 2.

However, I can see other scenarios getting more complex, so I am leaning on option 1

@seanrathier
Copy link
Contributor

seanrathier commented Jan 7, 2025

Is this proposal going to be resolved before the 8.18 release?
I have an open issue for validating the CSPM integration inputs that would need this and I will be willing to work on this once the solution decision is finalized.

CC: @acorretti @smriti0321

@kpollich
Copy link
Member

Is this proposal going to be resolved before the 8.18 release?

I can schedule this in our next sprint, but it's not clear how much work would be required to land this proposal in its current state or if it would need more work to solidify our approach.

Overall I'm in favor of option 2 because it requires less changes to the overall variable structure in packages. I think including "additional validation rules" at a higher level in the package will allow us to express logic like "at least one of these variables must be set" without tying it to a specific variable or requiring a full refactor of the variable declarations.

@kpollich
Copy link
Member

cc @jsoriano

@jsoriano
Copy link
Member Author

I also agree on option 2, specially because option 1 would be breaking for current packages and versions of Fleet.

However, I can see other scenarios getting more complex, so I am leaning on option 1

@seanrathier would you have examples of these more complex scenarios?

@kpollich kpollich assigned jsoriano and seanrathier and unassigned jsoriano Jan 13, 2025
@acorretti acorretti added this to the 9.0 milestone Jan 13, 2025
@seanrathier seanrathier modified the milestone: 9.0 Jan 13, 2025
@seanrathier
Copy link
Contributor

seanrathier commented Jan 14, 2025

@seanrathier would you have examples of these more complex scenarios?

@jsoriano, the use case for CSPM and CloudFlare are similar where this is a simple rule

(access_key && secret_key) || (session_key)

However, a more complex rule like below with option 1 would require us to duplicate the vars which may be an issue for the Fleet platform to change.

(varA && varB) || (varA && varC)

@jsoriano
Copy link
Member Author

However, a more complex rule like below with option 1 would require us to duplicate the vars which may be an issue for the Fleet platform to change.

(varA && varB) || (varA && varC)

Oh, good point, for this case option 2 would be better too, right?

That would be represented as:

required_vars:
  - [varA, varB]
  - [varA, varC]

@seanrathier
Copy link
Contributor

seanrathier commented Jan 15, 2025

Here is one of the complex validations I was referring to, if aws.account_type is organization-account we do not require any inputs because we deploy with a Cloud Formation template.

"cspm-cloudbeat/cis_aws": {
      "enabled": true,
      "vars": {
        "cloud_formation_template": "https://console.aws.amazon.com/cloudformation/home#/stacks/quickcreate?templateURL=https://elastic-cspm-cft.s3.eu-central-1.amazonaws.com/cloudformation-cspm-ACCOUNT_TYPE-8.17.0.yml&stackName=Elastic-Cloud-Security-Posture-Management&param_EnrollmentToken=FLEET_ENROLLMENT_TOKEN&param_FleetUrl=FLEET_URL&param_ElasticAgentVersion=KIBANA_VERSION&param_ElasticArtifactServer=https://artifacts.elastic.co/downloads/beats/elastic-agent",
        "cloud_formation_credentials_template": "https://console.aws.amazon.com/cloudformation/home#/stacks/quickcreate?templateURL=https://elastic-cspm-cft.s3.eu-central-1.amazonaws.com/cloudformation-cspm-direct-access-key-ACCOUNT_TYPE-8.17.0.yml",
        "cloud_formation_cloud_connectors_template": "https://console.aws.amazon.com/cloudformation/home#/stacks/quickcreate?templateURL=https://elastic-cspm-cft.s3.eu-central-1.amazonaws.com/cloudformation-cloud-connectors-ACCOUNT_TYPE-8.17.0.yml&param_ElasticResourceId=RESOURCE_ID"
      },
      "streams": {
        "cloud_security_posture.findings": {
          "enabled": true,
          "vars": {
            "access_key_id": "",
            "secret_access_key": "",
            "session_token": "",
            "role_arn": "",
            "aws.credentials.type": "cloud_formation",
            "aws.account_type": "organization-account"
          }
        }
      }

So I think we need an unless clause.

required_vars:
      required:
        - aws.credentials.type,
          aws.credentials.type,
          access_key_id,
          secret_access_key
      unless:
        - name: aws.credentials.type,
          value: cloud_formation,
        - name: aws.account_type,
          value: organization-account
required_vars:
      description: Required conditional variables for the package.
      type: object
      additionalProperties: false
      properties:
        required:
          description: Required variables for the package.
          type: array
          items:
            type: array
            items:
              type: string
              minItems: 1
        unless:
          description: >
            List of variables and the values that should defer validation.
          type: array
          items:
            type: object
            additionalProperties: false
            properties:
              name:
                description: Variable name.
                type: string
                examples:
                  - hosts
              value:
                description: Value of the variable.
                $ref: "#/definitions/input_variable_value"

@jsoriano
Copy link
Member Author

Here is one of the complex validations I was referring to, if aws.account_type is organization-account we do not require any inputs because we deploy with a Cloud Formation template.

Adding this unless looks like adding too much complexity here.

Couldn't this be represented like this?

required_vars:
- [access_key_id, secret_access_key]
- [access_key_id, secret_access_key, session_token]
- [shared_credential_file, credential_profile_name]
- [account_type]

Or if we want to be more explicit about some condition, maybe we can make the groups a list of objects instead of only the names, so we can require some values.

required_vars:
  secret_access_key:
    - name: aws.credentials.type
      value: secret_access_key
    - name: access_key_id
    - name: secret_access_key
  session_token:
    - name: aws.credentials.type
      value: secret_access_key
    - name: access_key_id
    - name: session_token
  credentials_file:
    - name: aws.credentials.type
      value: secret_access_key
    - name: shared_credential_file
    - name: credential_profile_name
  cloudformation:
    - name: aws.credentials.type
      value: cloud_formation
    - name: cloud_formation_template
    - name: cloud_formation_credentials_template
    - name: cloud_formation_cloud_connectors_template
  organization_account:
    - name: aws.account_type
      value: "organization-account"

Or we could build this as some kind of switch case, what would allow to define multiple sets of optional groups:

required_optional_vargroups:
  credential_type:
    variable: aws.credentials.type
    cases:
      secret_access_key: [access_key_id, secret_access_key]
      session_token: [access_key_id, session_token]
      credentials_file: [shared_credential_file, credential_profile_name]
      cloud_formation: [cloud_formation_template, cloud_formation_credentials_template, cloud_formation_cloud_connectors_template]
      organization_account: []
  some_other_option_id:
    variable: someotheroption
    cases:
      option1: [...]
      option2: [...]

@seanrathier
Copy link
Contributor

🤔 I like this approach, and this will work for complex validations

required_vars:
      description: Required conditional variables for the package.
      type: object
      additionalProperties: false
      patternProperties:
        "^[a-zA-Z0-9_]+$":
          type: array
          items:
            minItems: 1
            type: object
            required:
              - name
            additionalProperties: false
            properties:
              name:
                description: Required variable name
                type: string
                examples:
                  - hosts
              value:
                description: Value of the variable, if not present we validate the var's presense
                $ref: "#/definitions/input_variable_value"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issue needs discussion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants