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

Add Validate RPC to Config service SDK. #55

Merged
merged 5 commits into from
Jul 1, 2024
Merged

Conversation

edwbuck
Copy link

@edwbuck edwbuck commented Jun 28, 2024

Closes #53

// Required. True when the plugin deems the configuration usable.
bool valid = 1;

// Required. Text message suitable for presentation to end user.
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, what sort of information are you thinking a plugin would return in the valid case?

Copy link
Author

Choose a reason for hiding this comment

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

Minimally, "Configuration valid" or something like that. Really, it would depend on the plugin. Might indicate more, might indicate less.

For example, for the k8s_psat plugin, it might provide feedback that included the cluster name. For another plugin, it might echo back the configuration text.

There's not yet a distinct constraint on the message; but, having one alleviates the need to have variable logic on the success / failure cases, allowing the receiver to decide what to do without an if statement.

Copy link
Author

Choose a reason for hiding this comment

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

One idea that would have practical value is for the "success" return value to contain reminders to unblock ports, etc. Like I said, it would be plugin specific.

proto/spire/service/common/config/v1/config.proto Outdated Show resolved Hide resolved
Instead of passing one message, the API will pass back
one or more notes.  Each note is independent and will
provide feedback to the end user.

Signed-off-by: Edwin Buck <[email protected]>
@edwbuck
Copy link
Author

edwbuck commented Jun 29, 2024

@azdagron Sorry, I resolved your "notes" request, because I agreed with the change and pushed it. I'll keep it open next time so you can track it yourself, if you so desire.

@edwbuck edwbuck force-pushed the validate3 branch 2 times, most recently from a2e5ba6 to 2839960 Compare July 1, 2024 17:48
edwbuck and others added 2 commits July 1, 2024 12:59
Co-authored-by: Andrew Harding <[email protected]>
Signed-off-by: Edwin Buck <[email protected]>
Co-authored-by: Andrew Harding <[email protected]>
Signed-off-by: Edwin Buck <[email protected]>
@azdagron azdagron merged commit 594312f into spiffe:next Jul 1, 2024
4 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.

2 participants