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

Add e2e support for setting backend.id (Policy, Workflow, GraphQL) #624

Closed
Tracked by #604
mszostok opened this issue Jan 31, 2022 · 4 comments
Closed
Tracked by #604

Add e2e support for setting backend.id (Policy, Workflow, GraphQL) #624

mszostok opened this issue Jan 31, 2022 · 4 comments
Assignees
Labels
area/cli Relates to CLI area/documentation Related to all activities around documentation area/engine Relates to Engine area/hub Relates to Hub enhancement New feature or request
Milestone

Comments

@mszostok
Copy link
Member

mszostok commented Jan 31, 2022

Description

Update Policy syntax and related logic according to the Delegated Storage proposal.

Syntax

  • Breaking change:

    Before After
    rules:
    - interface:
        path: cap.interface.database.postgresql.install
        revision: 0.1.0 # exact revision
      oneOf:
        - implementationConstraints:
            # (...)
    interface:
      rules:  # rules for Interfaces, now nested under `interface`
      - interface:
          path: cap.interface.database.postgresql.install
          revision: 0.1.0 # exact revision
        oneOf:
          - implementationConstraints:
              # (...)
  • Enhancements:

    • TypeInstances policy:
    # ...
    
    typeInstance:
      rules:
        - typeRef:
            path: "cap.type.aws.auth.credentials"
            revision: "0.1.0" # optional
          backend:
            id: "00fd161c-01bd-47a6-9872-47490e11f996" # Vault backend storage
        - typeRef:
            path: "cap.type.aws.*" # for any Type reference starting with such prefix
          backend:
            id: "31bb8355-10d7-49ce-a739-4554d8a40b63" # AWS secrets manager
        - typeRef:
            path: "cap.*" # Any other Type reference
          backend:
            id: a36ed738-dfe7-45ec-acd1-8e44e8db893b
            description: "Default Capact PostgreSQL backend"

    if capact-outputTypeInstances[*].backend not specified, check default from Policy.

    # default - static
    capact-outputTypeInstances:
      - name: mattermost-config
        from: additional
        # no backend definition -> use default storage backend

    During render:

    typeInstances:
    - alias: helm-release
      attributes: []
      createdBy: default/act-l49vh-30c7a078-6a77-475c-94dd-7466f56447ce
      typeRef:
        path: cap.type.helm.chart.release
        revision: 0.1.0
      value: null
      backend:
        id: 3ef2e4ac-9070-4093-a3ce-142139fd4a16 # helm-release backend - resolved UUID based on the injected TypeInstance
    interface:
      rules:
        - interface:
            path: cap.interface.database.postgresql.install 
          oneOf:
            - implementationConstraints:
                  # constraints to select Bitnami PostgreSQL installation, for example:
                  path: cap.implementation.bitnami.postgresql.install
              inject:
                requiredTypeInstances:
                  - id: b4cf15d2-79b1-45ee-9729-6b83289ecabc # Different TypeInstance of `cap.type.helm.storage` Type - it will be used instead of the one from `interface.rules.default.inject`
                    description: "Helm Release storage"         
    
      default: # properties applied to all rules above
        inject:
          requiredTypeInstances:
          - id: "3ef2e4ac-9070-4093-a3ce-142139fd4a16"
            description: "Helm storage (cap.type.helm.storage:0.1.0)"

AC

  • Add new properties
  • Handle storage TypeInstance injections
  • Update documentation

Extra scope:

  • update renderer logic
  • update local GraphQL schema
  • update engine GraphQL schema
  • update engine logic
  • update engine CRD
  • update Dashboard
  • set proper uses relations between storage backend TypeInstance and other TypeInstances

Related issues

See epic #604 for reason and use cases.

@mszostok mszostok added this to the 0.7.0 milestone Jan 31, 2022
@mszostok mszostok added area/cli Relates to CLI area/documentation Related to all activities around documentation area/engine Relates to Engine area/hub Relates to Hub enhancement New feature or request labels Jan 31, 2022
@mszostok mszostok self-assigned this Jan 31, 2022
@mszostok mszostok changed the title Update Policy API and PolicyEnforcedClient [5MD] Update Policy API and PolicyEnforcedClient Jan 31, 2022
@mszostok
Copy link
Member Author

mszostok commented Jan 31, 2022

@mszostok
Copy link
Member Author

mszostok commented Feb 13, 2022

Findings

Personally, I discovered two things that I don't like:

  • For validation purposes, we need to know if requiredTypeInstances are backend or not during the render process.
  • We don't need to inject the requiredTypeInstance if it's a backend.

IMO it would be good to revisit the whole solution after implementing that and adjust where needed (external feedback would be very helpful). Maybe we should introduce dedicated field for injecting storage backend instead of using requires section. But it's too early to go with such conclusions.

Follow-ups

  1. Update generators and linters: Bump Go and Kubernetes dependencies #611 (comment)
  2. Prevent TypeInstance deletion when it's used by others #632
  3. Add new item to delegated storage epic:
    • Validate Type storage schema convention. In case of the cap.core.hub.storage additional reference, we should prevent uploading such Type if the JSON schema don't meet our conditions.
  4. Make Local Hub aware of Public Hub and validate TypeInstances #633 (Validate TypeRef on Local Hub.)

Changes

  • I removed setting the built-in storage, and relations in renderer as suggested on proposal. Now they are handled by Local Hub.
    IMO it's much better approach as it is always needed to resolve data properly and in the future support the Uninstalling storage backends proposal. If we will require that on TypeInstance creation, then user may easily forget about it.

  • There were two “approaches” in proposal for backend field. One to give an option to update it and second mark it as immutable property. To simplify the current implementation, I decided to go with an immutable approach. This can be easily changed by follow-up PR, where change will be better visible and also better tested.

Notes

Workflow policy doesn't have an option to specify the TypeInstance backends for TypeRefs as this serves as default mechanism both for Global and Action Policy only.

HINT: The TypeInstanceBackend in k8s GraphQL was changed to TypeInstanceBackendRule to overcome problem with conflict on Gateway side. Same with the Input.

Questions

  • Should we ignore injecting required Types with aliases, which are storage backends? Currently, injecting them is unnecessary.

@mszostok mszostok changed the title Update Policy API and PolicyEnforcedClient Add e2e support for setting backend.id (Policy, Workflow, GraphQL) Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Relates to CLI area/documentation Related to all activities around documentation area/engine Relates to Engine area/hub Relates to Hub enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant