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

Update agents listing service with "CanReAttest" filter #3880

Conversation

guilhermocc
Copy link
Contributor

@guilhermocc guilhermocc commented Feb 17, 2023

Pull Request check list

  • Commit conforms to CONTRIBUTING.md?
  • Proper tests/regressions included?
  • Documentation updated?

Affected functionality
spire-server agents listing.

Description of change

  • Add "CanReAttest" boolean filter in list agents method
  • Update spire-server agent and entry commands to add new response fields: registration entry hint and agent canReAttest.

Which issue this PR fixes
Ongoing work for #1836

Dependencies
⚠️ This PR depends on spiffe/spire-api-sdk#36. After it's merge, go mod file should be updated to remove temporary local spire-api-sdk reference. ⚠️

@faisal-memon
Copy link
Contributor

I can review

@MarcosDY MarcosDY self-assigned this Feb 23, 2023
@evan2645 evan2645 added this to the 1.6.1 milestone Feb 28, 2023
@@ -266,7 +266,7 @@ func TestList(t *testing.T) {
expectedReturnCode: 0,
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you add a test case where an agent that is re_attestable is returned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -289,6 +289,7 @@ StoreSvid : true
},
"entry": {
"id": "entry-id",
"hint": "",
Copy link
Collaborator

Choose a reason for hiding this comment

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

why hint is required here? looks like it is not related with this PR (the same comment is for all files with the same change)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The new version of spire-api-sdk comes with the hint field change; that's why I had to include those here. This field will be effectively included in #3926

@guilhermocc
Copy link
Contributor Author

I recommend reviewing #3926 before this PR, since it will include the "hint" field which will also be added with the new spire-api-sdk version required by this PR.

@guilhermocc guilhermocc force-pushed the update-agents-listing-service-with-new-filter branch from 59baf1f to 659d2fc Compare March 8, 2023 13:36
Signed-off-by: Guilherme Carvalho <guilhermbrsp@gmail.com>
Signed-off-by: Guilherme Carvalho <guilhermbrsp@gmail.com>
Signed-off-by: Guilherme Carvalho <guilhermbrsp@gmail.com>
Signed-off-by: Guilherme Carvalho <guilhermbrsp@gmail.com>
Signed-off-by: Guilherme Carvalho <guilhermbrsp@gmail.com>
@guilhermocc guilhermocc force-pushed the update-agents-listing-service-with-new-filter branch from 659d2fc to 4b05585 Compare March 8, 2023 13:55
Signed-off-by: Guilherme Carvalho <guilhermbrsp@gmail.com>
Signed-off-by: Guilherme Carvalho <guilhermbrsp@gmail.com>
@guilhermocc guilhermocc force-pushed the update-agents-listing-service-with-new-filter branch from 4bd9a7e to 25f6a60 Compare March 8, 2023 19:12
@MarcosDY MarcosDY merged commit 6aa3c8b into spiffe:main Mar 9, 2023
@amartinezfayo amartinezfayo modified the milestones: 1.6.2, 1.6.3 Apr 5, 2023
moe-omar pushed a commit to moe-omar/spire that referenced this pull request Jul 20, 2023
- Add "CanReAttest" boolean filter in list agents method
- Update spire-server agent and entry commands to add new response fields: registration entry hint and agent canReAttest.

---------

Signed-off-by: Guilherme Carvalho <guilhermbrsp@gmail.com>
Signed-off-by: mo omar <momar@confluent.io>
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.

5 participants