test: Update Trustee deployment to match the simplifed deployment overlays #2099
Conversation
stevenhorsman
force-pushed
the
non-se-kbs-deployment
branch
from
f2be871
to
47a89e9
Compare
stevenhorsman
force-pushed
the
non-se-kbs-deployment
branch
2 times, most recently
from
e6a9176
to
46ae2c7
Compare
stevenhorsman
force-pushed
the
non-se-kbs-deployment
branch
4 times, most recently
from
3a6c506
to
5964eef
Compare
stevenhorsman
force-pushed
the
non-se-kbs-deployment
branch
from
5964eef
to
f7e4d1b
Compare
| if err != nil { | ||
| return err | ||
| } | ||
| keyFilePath := "../../kbs/config/kubernetes/overlays/" + platform + "/key.bin" | ||
| keyFilePath := "../../kbs/config/kubernetes/" + overlaysPath + "/key.bin" |
There was a problem hiding this comment.
Is there any way to use absolute path for privateKey and keyFilePath based on the TRUSTEE_REPO_PATH?
There was a problem hiding this comment.
I started writing this code, but then noticed const TRUSTEE_REPO_PATH = "../trustee", so the TRUSTEE_PATH isn't absolute. I'm happy to continue the refactor to reduce the number of relative paths we have though?
There was a problem hiding this comment.
I've created 4281c1e - I'm not sure how happy I am with it, so feel free to give critical feedback and suggestions!
There was a problem hiding this comment.
It looks like the refactor might have broken the tests too, so I'll look into this locally
There was a problem hiding this comment.
Based on your comment, it looks like a general refactor is needed to use absolute path. How about you merge this and handle the refactor as a separate PR ?
There was a problem hiding this comment.
Yeah, I think this commit does help reduce relative paths of relatives paths (in the old code we'd have something like ../../trustee/../../<file>) so we just have the single relative path here now to ../trustee from test/e2e, but there is room for improvement later. I think I already have 1/2 refactors in this area already, so we can steadily get it better.
There was a problem hiding this comment.
I decided to just take on the battle here, so have re-work the commit to remove the relative trustee path too. It relies on NewKeyBrokerService to set it up initially, but as all the references to the trustee path are in that func, or methods of KeyBrokerService I think we'll be okay.
stevenhorsman
force-pushed
the
non-se-kbs-deployment
branch
from
f7e4d1b
to
4281c1e
Compare
Bump to pick up the commit with the s390x changes supported Signed-off-by: stevenhorsman <steven@uk.ibm.com>
- Update deployments and add support for non-SE s390x KBS deployment based on the updated trustee deployment overlays - Make uname command compatible: `uname -i` doesn't seem to work on all platforms, so switch to `uname -m` which seems to be portable and gives the same result on ubuntu x86 & s390x Signed-off-by: stevenhorsman <steven@uk.ibm.com>
Refactor to reduce usage of relative paths Signed-off-by: stevenhorsman <steven@uk.ibm.com>
stevenhorsman
force-pushed
the
non-se-kbs-deployment
branch
from
337a2a3
to
e031458
Compare
|
FYI the linting failure is (I believe) caused by github updating their default runner, so I have try to address this in #2120 |
In confidential-containers/trustee#521 I did some work to enable Trustee to work on s390x (without SE) and generally simplify deployments, such that overlays are only needed for ibm-se, not for x86/s390x in general.
This PR adapts us to use this approach.