Switch KMIP testing to cosmian#575
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (76.27%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #575 +/- ##
=======================================
Coverage 57.67% 57.67%
=======================================
Files 68 68
Lines 10758 10758
Branches 2650 2650
=======================================
Hits 6205 6205
- Misses 3279 3280 +1
+ Partials 1274 1273 -1
🚀 New features to boost your workflow:
|
dutow
force-pushed
the
kmip-cosmian-tap
branch
3 times, most recently
from
deb2bb8 to
59f03d4
Compare
|
|
||
| # Cosmian KMS | ||
| wget https://package.cosmian.com/kms/5.21.0/deb/$(dpkg --print-architecture)/non-fips/static/cosmian-kms-server-non-fips-static-openssl_5.21.0_$(dpkg --print-architecture).deb | ||
| sudo dpkg -i cosmian-kms-server-non-fips-static-openssl_5.21.0_$(dpkg --print-architecture).deb |
There was a problem hiding this comment.
Should we add a variable for the architecture?
There was a problem hiding this comment.
Would that be more reliable than calling dpkg?
There was a problem hiding this comment.
No, my idea was jsut to reduce duplicated calls to dpkg.
jeltz
left a comment
jeltz
left a comment
There was a problem hiding this comment.
I cannot say I was happy with doing this review. Feels like it would have needed quite a bit more work before it would actually have been ready for review.
| diag("COSMIAN_KMS_BIN=$override is not executable; ignoring"); | ||
| } | ||
|
|
||
| for my $dir (File::Spec->path) |
There was a problem hiding this comment.
This seems overkill. Why not just check if it is in the current path?
There was a problem hiding this comment.
Because perl doesn't have a built in for that, a short if seems a good alternative to me vs adding a new package dependency
|
|
||
| # Cosmian needs the openssl legacy provider to read the P12 bundle. | ||
| # Search known module dirs across distros; on Ubuntu CI the post-install | ||
| # chmod in ci_scripts/ubuntu-deps.sh exposes cosmian's bundled copy. |
There was a problem hiding this comment.
I don't really get why the code below is necessary and I do not really feel the comment explains it.
There was a problem hiding this comment.
Yeah, maybe I could make this comment better, basically cosmian-kms needs OPENSSL_MODULES set
| # is present (the Ubuntu .deb installs /etc/cosmian/kms.toml and | ||
| # cosmian's `-c` flag's help text says "All other command line | ||
| # arguments and environment variables are ignored once the | ||
| # configuration file is loaded"). Always launch via -c <our toml>. |
There was a problem hiding this comment.
This comment adds no value as far as I can see.
There was a problem hiding this comment.
Hm, maybe. I left it there because this was really annoying
There was a problem hiding this comment.
I don't really see how this comment adds anything. It is annoying but not really anything we need to document since they already do it.
dutow
force-pushed
the
kmip-cosmian-tap
branch
2 times, most recently
from
120ae67 to
8c80cbb
Compare
PyKMIP seems to be abandonware and requires an older python version. This commt switches instead to use a Cosmian based kmip test, which also moves the server setup inside the TAP test to simplify local deployment. Test assumes that the required binaries are installed and are user executable.
Verify pg_tde fails fast when KMIP endpoint accepts TCP but immediately closes (no TLS, no KMIP). Uses a fork()ed accept-then-close listener on an ephemeral port, expecting the provider registration to surface an SSL/handshake/EOF error rather than hanging.
dutow
force-pushed
the
kmip-cosmian-tap
branch
2 times, most recently
from
796d8c8 to
2849472
Compare
3 participants
PyKMIP seems to be abandonware and requires an older python version. This commt switches instead to use a Cosmian based kmip test, which also moves the server setup inside the TAP test to simplify local deployment.
Test assumes that the required binaries are installed and are user executable.
(Should we use something other than perl for testing?)