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

CLI: specify ATTESTER to build kbs-client #429

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

genjuro214
Copy link
Contributor

@genjuro214 genjuro214 commented Jun 25, 2024

We can't build kbs-client on s390x host with default feature including all attesters.

I tried to add a new variable ATTESTER in Makefile, so that we can specify separate attester when building kbs-client

  • by default, make cli will build kbs-client with all attesters
  • keep feature sample_only: CLI_FEATURES=sample_only make cli
  • specify ATTESTER: ATTESTER=se-attester make cli

@huoqifeng
Copy link
Contributor

se-only feature tag looks weird to me, maybe, considering to add ATTESTER like guest-components? like https://github.com/confidential-containers/guest-components/blob/main/attestation-agent/Makefile#L28

- Add ATTESTER in Makefile to specify seperate attester
  for kbs-client

Signed-off-by: Lei Li <[email protected]>
Signed-off-by: Lei Li <[email protected]>
@genjuro214 genjuro214 changed the title ibmse: add se_only feature for kbs-client CLI: specify ATTESTER for to build kbs-client Jun 25, 2024
@huoqifeng
Copy link
Contributor

@Xynnn007 @fitzthum @arronwy may you help have a look?

@genjuro214 genjuro214 changed the title CLI: specify ATTESTER for to build kbs-client CLI: specify ATTESTER to build kbs-client Jun 25, 2024
Copy link
Contributor

@huoqifeng huoqifeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine.

@@ -20,6 +21,14 @@ else
AS_FEATURE = $(AS_TYPE)
endif

ifndef CLI_FEATURES
ifdef ATTESTER
CLI_FEATURES = "sample_only,$(ATTESTER)"
Copy link
Member

@fitzthum fitzthum Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit misleading to use both sample_only and another attester, but I guess we can live with that. I think we depend on this sample_only feature externally so we probably don't want to rename that.

@fitzthum fitzthum merged commit b9ab551 into confidential-containers:main Jun 27, 2024
15 checks passed
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