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

Allow DelegatedIdentity API clients to subscribe by PID #5272

Merged
merged 21 commits into from
Aug 14, 2024

Conversation

bleggett
Copy link
Contributor

@bleggett bleggett commented Jul 2, 2024

Pull Request check list

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

Affected functionality
DelegateIdentity API

Description of change
This is the impl side of spiffe/spire-api-sdk#58

For this to build you need to locally check out the spire-api-sdk fork @40d1bc186c2da087756115d9e6eb45e75aef0b3b as a sibling of this repo, so until/if that is merged, this should not be.

CI will not pass until ☝️ happens!

Ended up being simpler than I expected.

Note that docs and examples are pending still.

Which issue this PR fixes
#5019

Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
@bleggett bleggett changed the title Allow DelegatedIdentity API clients to subscribe by PID WIP: Allow DelegatedIdentity API clients to subscribe by PID Jul 2, 2024
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
@bleggett bleggett changed the title WIP: Allow DelegatedIdentity API clients to subscribe by PID Allow DelegatedIdentity API clients to subscribe by PID Jul 3, 2024
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
@bleggett bleggett force-pushed the bleggett/pids-thru-delegate-api branch from fd38b74 to c2f2f2c Compare July 3, 2024 19:26
bleggett and others added 9 commits July 19, 2024 18:27
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
@bleggett
Copy link
Contributor Author

@azdagron et al this end is now passing CI - PTAL at the spire_agent.md docs especially, but this end should be G2G as well now.

@bleggett
Copy link
Contributor Author

bleggett commented Jul 25, 2024

(WAS passing until I resynced with main - that windows unit test seems like an unrelated flake tho - it was passing before)

Copy link
Member

@azdagron azdagron left a comment

Choose a reason for hiding this comment

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

Looks pretty straightforward. Just missing a few test cases. Thanks for the work, @bleggett!

pkg/agent/api/delegatedidentity/v1/service.go Outdated Show resolved Hide resolved
pkg/agent/api/delegatedidentity/v1/service.go Outdated Show resolved Hide resolved
@azdagron
Copy link
Member

Hey @bleggett! Just a friendly ping about addressing the comments. I think we're pretty close here!

bleggett and others added 3 commits August 13, 2024 19:41
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
@bleggett
Copy link
Contributor Author

@azdagron everything should be addressed, lmk if you have further comments, ty!

Signed-off-by: Benjamin Leggett <benjamin.leggett@solo.io>
Copy link
Member

@azdagron azdagron left a comment

Choose a reason for hiding this comment

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

\o/

@azdagron azdagron merged commit 9f002a4 into spiffe:main Aug 14, 2024
34 checks passed
@bleggett
Copy link
Contributor Author

ty all!

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.

3 participants