Skip to content

Conversation

@jakedoublev
Copy link
Contributor

@jakedoublev jakedoublev commented Nov 20, 2024

  1. align flags throughout KAS registry commands (i.e. --public-key should not have different shorthands, --id as -i should be consistent throughout all commands)
  2. improve code readability
  3. improve CLI e2e coverage of KAS registry CRUD

@jakedoublev jakedoublev requested a review from a team as a code owner November 20, 2024 18:56
@jakedoublev jakedoublev enabled auto-merge (squash) November 21, 2024 21:19
@jakedoublev jakedoublev changed the title fix(core): kas registry get should allow -i 'id' flag shorthand fix(core): kas registry commands UX improvements Nov 21, 2024
@jakedoublev
Copy link
Contributor Author

@dmihalcik-virtru is it accurate that the KAS key pem should not be base64 encoded? That appears to be the case in our x-tests: https://github.com/opentdf/tests/blob/450ca7ffef8656eae765e1b62795a24e860d678b/xtest/conftest.py#L102

@dmihalcik-virtru
Copy link
Member

The KAS kas_public_key REST endpoint takes a fmt parameter which can be used to control how it is returned. IIRC it currently maybe handles raw and pkcs8 (or maybe 12 for certs)? Maybe you can do CERTIFICATE or PUBLIC KEY types? We probably should be storing PKCS8 in the database, it probably doesn't matter if they are also pem encoded or not and i could see the value either way (less logic for serving PEM encoded variants, which is what most clients will want - at least until we transition to JWK, but more efficient/more direct to store the ASN.1 binary directly).

@jakedoublev
Copy link
Contributor Author

Thanks @dmihalcik-virtru. With the underlying structure managed by the platform and the otdfctl CLI as a simple client, it sounds like the solution for now is to not base64 encode the public key of a registered KAS in these CLI e2e tests or examples.

@jakedoublev jakedoublev merged commit bed3701 into main Dec 3, 2024
9 checks passed
@jakedoublev jakedoublev deleted the fix/kasr-get-i branch December 3, 2024 15:12
jakedoublev pushed a commit that referenced this pull request Dec 5, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.17.0](v0.16.0...v0.17.0)
(2024-12-05)


### Features

* **core:** pagination of LIST commands
([#447](#447))
([673a064](673a064))
* **core:** subject condition set prune
([#439](#439))
([c4c8b8b](c4c8b8b))


### Bug Fixes

* **core:** kas registry get should allow -i 'id' flag shorthand
([#434](#434))
([bed3701](bed3701))
* **core:** sm list should provide value fqn instead of just value
string ([#438](#438))
([9a7cb72](9a7cb72))

---
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.

4 participants