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

new capability to set a platform mode #1033

Closed
strantalis opened this issue Jun 25, 2024 · 5 comments · Fixed by #1245
Closed

new capability to set a platform mode #1033

strantalis opened this issue Jun 25, 2024 · 5 comments · Fixed by #1245
Assignees
Labels
adr Architecture Decision Records pertaining to OpenTDF comp:core Core component enhancement New feature or request

Comments

@strantalis
Copy link
Member

strantalis commented Jun 25, 2024

Currently, when running the platform, we need to explicitly set whether a service is enabled. This works, but as we start to look at running a service like KAS independently, we need to consider how to manage this configuration more efficiently. It will become cumbersome if every service within the platform needs to be individually enabled or disabled.

Additionally, not every service requires the same tooling. For example, KAS does not need opa, yet opa is created regardless of the platform configuration.

logger.Info("starting opa engine")
eng, err := opa.NewEngine(conf.OPA)
if err != nil {
return fmt.Errorf("could not start opa engine: %w", err)
}
defer eng.Stop(ctx)

First, we need to define what constitutes the core platform.

Core Platform:

  • Well-known (Platform Discovery) Endpoint
  • Policy

The two other services that aren't part of the core are:

Authorization Service
Key Access Service (KAS)

If we agree that these are the main services, we can define the following modes:

  • all - Runs all services as a monolith
  • core
  • authorization
  • kas

As users potentially need to extend or build on top of OpenTDF they could also potentially need to define their own modes as well. This gives us the ability to provide that. So as they add new services under services that will essentially become a mode.

Current configuration looks something like this where each service is explicitly defined under the services block.

services:
kas:
enabled: true
keyring:
- kid: e1
alg: ec:secp256r1
- kid: e1
alg: ec:secp256r1
legacy: true
- kid: r1
alg: rsa:2048
- kid: r1
alg: rsa:2048
legacy: true
policy:
enabled: true
authorization:
enabled: true
ersurl: http://localhost:8080/entityresolution/resolve
clientid: tdf-authorization-svc
clientsecret: secret
tokenendpoint: http://localhost:8888/auth/realms/opentdf/protocol/openid-connect/token
entityresolution:
enabled: true
url: http://localhost:8888/auth
clientid: "tdf-entity-resolution"
clientsecret: "secret"
realm: "opentdf"
legacykeycloak: true

In the new approach, the enabled field would be removed as everything would be driven by the mode.

https://github.com/opentdf/platform/blob/main/service/pkg/serviceregistry/serviceregistry.go#L22

We wouldn't be required to set defaults for those services like we do today.

https://github.com/opentdf/platform/blob/main/service/internal/config/config.go#L27

Example All Mode

mode: all
credentials:
  client_id: xxxxxx
  client_secret: xxxxx
db:
  <db config>
services:
  kas:
   <any kas config>
  core:
    <any core config>
  authorization:
    <an authorization config>

Example Core Mode

mode: core # Not sure if we want to call this mode something else?
credentials:
  client_id: xxxxxx
  client_secret: xxxxx
db:
  <db config>
services:
  core:
    <any core config >

Example KAS Mode

mode: kas
credentials:
  client_id: xxxxxx
  client_secret: xxxxx
remote_services:
  core: https://core.opentdf.io:9000
services:
  kas:
    <any core config>

As an alternative to having a root-level remote_services block, we could also consider the following configuration, although it might be more confusing:

mode: kas
credentials:
  client_id: xxxxxx
  client_secret: xxxxx
services:
  kas:
    core_endpoint: https://core.opentdf.io:9000
    <any core config>
@jakedoublev
Copy link
Contributor

This looks wise. I have a couple questions:

  1. Is credentials a new addition, or a relocation of the auth service client id and secret config?

    clientid: tdf-authorization-svc
    clientsecret: secret

  2. Should we make modes extendable for downstream PEPs?

If the platform has a new service added to its core (i.e. new-core-service), as a downstream PEP developer my core mode could extend via hook into whatever services the platform considers core rather than duplicating policy, well-known, new-core-service, my-core-pep-service. It seems like there'd be fewer configuration changes needed when upgrading.

@strantalis
Copy link
Member Author

  1. Is credentials a new addition, or a relocation of the auth service client id and secret config?
    clientid: tdf-authorization-svc
    clientsecret: secret

Yeah, I kind of snuck it in there. Right now, each service has its own credentials config. But when we're running everything together, it doesn’t really make sense for each service to have its own separate credentials. Plus, using client credentials from the IDP means we’d need to create at least three new clients right now.

By defining a global credentials block, we can use it across the board, whether we're in mode all or running a service on its own.

@strantalis
Copy link
Member Author

  1. Should we make modes extendable for downstream PEPs?

If the platform has a new service added to its core (i.e. new-core-service), as a downstream PEP developer my core mode could extend via hook into whatever services the platform considers core rather than duplicating policy, well-known, new-core-service, my-core-pep-service. It seems like there'd be fewer configuration changes needed when upgrading.

I think they will be extensible if we keep Services as a map.

Services map[string]serviceregistry.ServiceConfig `yaml:"services" default:"{\"policy\": {\"enabled\": true}, \"health\": {\"enabled\": true}, \"authorization\": {\"enabled\": true}, \"wellknown\": {\"enabled\": true}, \"kas\": {\"enabled\": true}, \"entityresolution\": {\"enabled\": true}}"`

Each key in the map could be considered a mode is how I was thinking about it.

I think we probably need to try and implement this to really shake the config proposal changes out.

@jrschumacher
Copy link
Member

In my mind the core is the service which you reach out to for networking info. So if a PEP is running in the core that is fine but it will just use the SDK to reach out to the other systems and the platform will negotiate that based on the remotes.

If you run the PEP in a microservice it will require knowledge of the core, but that's it. Might be a bit costly to call the core to then be directed to a remote KAS but that's fine for now. Less complexity than having to manage all the URLs of every microservice deployment.

@strantalis
Copy link
Member Author

In my mind the core is the service which you reach out to for networking info. So if a PEP is running in the core that is fine but it will just use the SDK to reach out to the other systems and the platform will negotiate that based on the remotes.

If you run the PEP in a microservice it will require knowledge of the core, but that's it. Might be a bit costly to call the core to then be directed to a remote KAS but that's fine for now. Less complexity than having to manage all the URLs of every microservice deployment.

I don’t think it will be expensive. We already call the platform when creating a new client to get IDP information, and there are always optimizations we can implement in the future.

The only other static endpoint, in my mind, would be the authorization service. KAS endpoints will be more dynamic, driven by core policy, so clients will have to reach out to get a full attribute value definition.

@jrschumacher Are you suggesting we create a root-level config for core_endpoint or platform_endpoint?

@strantalis strantalis self-assigned this Jun 28, 2024
@strantalis strantalis added enhancement New feature or request comp:core Core component labels Jun 28, 2024
github-merge-queue bot pushed a commit that referenced this issue Jul 9, 2024
This pull request helps address
#1033 and introducing the
concept of a mode.

Currently we are starting `opa` for every service. When in practice it
is only consumed by the authorization service that is dependent on
executing the `rego` policy.

In this update, I have removed OPA and instead directly eval the Rego
policy, which can be either embedded or passed in via configuration.
Until we have clear use cases for the additional management
functionality that OPA provides, we can focus on just executing our
entitlements policy.
@strantalis strantalis added the adr Architecture Decision Records pertaining to OpenTDF label Jul 29, 2024
github-merge-queue bot pushed a commit that referenced this issue Aug 12, 2024
This pr resolves #1033

It introduces two new config values `mode` and `sdk_config` at the root
level. It also removes the `enabled` field on a service.

Mode now takes a list which could be `all`, `core` or a specific
namespaced service for example `kas`. You can also combine modes like
`core,kas`.

When running in something other than `core` or `all` you are required to
specify a `sdk_config` object. This is because the core services will be
remote now and not over the internal IPC server.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adr Architecture Decision Records pertaining to OpenTDF comp:core Core component enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants