Skip to content

Conversation

@strantalis
Copy link
Member

@strantalis strantalis commented Aug 1, 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.

@strantalis strantalis changed the title Impl mode feat(core): ability to run a set of isolated services Aug 1, 2024
@strantalis strantalis marked this pull request as ready for review August 3, 2024 20:15
@strantalis strantalis requested review from a team as code owners August 3, 2024 20:15
@strantalis strantalis requested a review from jakedoublev August 5, 2024 20:09
@pflynn-virtru
Copy link
Member

suite

@dmihalcik-virtru
Copy link
Member

Is the migration stuff part of this PR or could it be moved to its own?

Also, will people need to update their config files after this change (what breakages if any could exist when using an old config file with this one)?

@jrschumacher
Copy link
Member

I think we should describe what we expect to happen.... and on this note maybe we need to add config versions so we can detect changes in the future? This won't work for config maps.

  1. Without Mode the platform should fail to start and make note of the recent changes
  2. If Services still have Enabled we should ignore unless validation exists

There are libs that validate yaml files and refuse to function if they don't meet the schema. For instance, WebPack and Rollup in JS land. Would this be critical to add?

@jrschumacher
Copy link
Member

Is the migration stuff part of this PR or could it be moved to its own?

@dmihalcik-virtru this refactor is really focused on the serviceregistry since the modes impacts the way that was implemented.

Sean brought me in during his refactor (pair programming) to check with me on the original implementation. I agree with this refactor since it's more clear. Also with respect to OpenTDF Policy is the only service that uses a DBClient, and we have extensive integration tests that are passing.

If I missed a specific section you're concerned about, would you point that out to me?

@strantalis
Copy link
Member Author

@jrschumacher @dmihalcik-virtru The default mode is all. In opentdf I don't think we ever disable any services that come out of the box. So for now its an upgrade documentation exercise for our internal product. I haven't even begun to think about versioning the config and what complexities that brings versus just have really detailed upgrade notes.

@strantalis strantalis requested a review from jrschumacher August 9, 2024 19:57
@jrschumacher
Copy link
Member

I agree. Default should be monolith, and we can state that this is not a recommendation for a secure production environment. Additionally, I don't think OpenTDF as a group should be offering production advice, since that typically involves liability and insurance.

@strantalis strantalis enabled auto-merge August 12, 2024 18:43
@strantalis strantalis added this pull request to the merge queue Aug 12, 2024
Merged via the queue into opentdf:main with commit aa5636a Aug 12, 2024
@strantalis strantalis deleted the impl-mode branch August 12, 2024 19:02
github-merge-queue bot pushed a commit that referenced this pull request Aug 12, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.4.18](service/v0.4.17...service/v0.4.18)
(2024-08-12)


### Features

* **authz:** Remove external ers configuration from authorization
([#1265](#1265))
([aa925a8](aa925a8))
* **authz:** Typed Entities
([#1249](#1249))
([cfab3ad](cfab3ad))
* **core:** ability to run a set of isolated services
([#1245](#1245))
([aa5636a](aa5636a))
* **core:** improve entitlements performance
([#1271](#1271))
([f6a1b26](f6a1b26))
* **core:** policy support for LIST of kas grants (protos/db)
([#1317](#1317))
([599fc56](599fc56))
* **core:** Simplifies support for kidless clients
([#1272](#1272))
([dedeb32](dedeb32))
* **policy:** 1256 resource mapping groups db support
([#1270](#1270))
([c020e9b](c020e9b))
* **policy:** 1277 add Resource Mapping Group to objects proto
([#1309](#1309))
([514f1b8](514f1b8)),
closes [#1277](#1277)


### Bug Fixes

* **core:** Autobump service
([#1322](#1322))
([9460fb5](9460fb5))
* **core:** casbin policy should support assign/remove/deactivate rpc
naming ([#1298](#1298))
([288921b](288921b)),
closes [#1303](#1303)
* **core:** put back proto breaking change detection in CI
([#1292](#1292))
([9921962](9921962)),
closes [#1293](#1293)
* **core:** Update casbin policy for rewrap with unknown role
([#1305](#1305))
([de5be3c](de5be3c))
* **policy:** deprecates and reserves value members from value object in
protos ([#1151](#1151))
([07fcc9e](07fcc9e))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

new capability to set a platform mode

5 participants