-
Notifications
You must be signed in to change notification settings - Fork 58
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
Adding /agent/info API to agent #758
Conversation
The feature seems straightforward. About the test issues, it seems the nightly rust compiler changed and is now failing to compile. Let's see if that gets fixed "automagically" or if we will have to make changes to the compiler, probably pinning the version with the coverage measurement support instead of using the nightly version. The formatting issue normally can be fixed automatically by running |
0ed81b5
to
524a6b8
Compare
Thanks, done. |
I hope this fixes the CI issues: RedHat-SP-Security/keylime-tests#560 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Should the API version be bumped due to this addition? |
Yes. I don't know what process we want for that. Do we want to bump the version when we make a single change? Or before we do a release? I'm open to either one. |
/packit retest-failed |
I think the best would be to bump the version with the first API change made in the release, but do a single bump including all the changes made to the API per release. This would better test the bump itself during the release development. It would be risky and error prone to bump the version only when creating the new release. |
@ansasaki It's an interesting problem with the version numbers: If I bump them in the code in the agent not only will the agent's URLs be /v2.2 but it will be trying to use the /v2.2 APIs of the registrar and verifier which don't yet exist. Now in my case I do have a companion PR (keylime/keylime#1532) on the server side so I'll go ahead and bump up the versions there too... but what do we do if it's just a change in the agent's APIs? Do we have to have a "dummy" PR in the server side to bump up the APIs first there before we can do it in the agent? |
For now, I think the easiest way would be to add the new supported version there and then bump the version here. We should discuss with the community what will be the process, and also think if it is worth to increase the complexity of the agent to add more than one supported API version. We agreed in the past that the agent would provide only the latest API version, while the python components would handle the various possible supported agent API's. I also hit this problem when updating some tests and created an issue to track this: #790 We should consider dissociating the API versions, but that will come with the need to manage on the agent side which versions of the registrar API are supported. Not an easy decision. |
cb0c543
to
641cfc1
Compare
/packit retest-failed |
4 similar comments
/packit retest-failed |
/packit retest-failed |
/packit retest-failed |
/packit retest-failed |
@kkaarreell It seems the new version of the coverage tool @mpeters There are warnings that are generated by the new rust compiler, I proposed a fix for them in #793. Meanwhile, the tests in the CI will probably continue to fail. I tested your code manually on top of #793 and the test you added passes. |
/packit retest-failed |
/packit retest-failed |
I think that the build on F39 is failing due to some repodata (cache) issue. It is installing old i686 version of tpm2-tss-devel from fedora repo instead of installing it from updates repo.
Moreover, on my F39 test system I have already 4.0.2-1 in updates. |
You could try editing |
@kkaarreell Is this before or after the calls to disable |
Bumping API version to 2.2 Signed-off-by: Michael Peters <mpeters@redhat.com>
Hm, it seems it didnt help, I'm sorry. Weird. |
~~ I think I know whats going on. F39 image Is old and already contains old tpm2-tss package. Therefore the -devel subpackage gets installed in the same old version. Either we would have to explicitly update or ask Testing Farm to create newer image. I will do the later tomorrow for sure. ~~ EDIT: I will consult TF folks tomorrow. I have reserved a system from TF and it can install tpm2-tss-devel with the right version and architecture.
|
Hi Michael, try rescheduling it once more. In #795 CI has passed for me even without an explicit makecache call. |
/packit retest-failed |
This is a new read-only API on the agent to allow for 3rd party integrations (starting with SPIRE) that will need certain information about the agent.