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 implementation for Namespace related GRPC methods in resources plugin #3910

Merged
merged 2 commits into from
Dec 10, 2021

Conversation

absoludity
Copy link
Contributor

Signed-off-by: Michael Nelson minelson@vmware.com

Description of the change

Follows on from #3905 , adding the implementation of those namespace GRPC methods of the resource plugin.

Benefits

The next PR will be for the dashboard and can remove the calls to the K8s API server for the namespace functionality.

Possible drawbacks

Applicable issues

Additional information

@@ -416,13 +416,16 @@ func resourceRefsEqual(r1, r2 *pkgsGRPCv1alpha1.ResourceRef) bool {

// errorByStatus generates a meaningful error message
func errorByStatus(verb, resource, identifier string, err error) error {
// If the error is already a k8s StatusError, return the StatusErr
Copy link
Collaborator

Choose a reason for hiding this comment

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

@absoludity Is this a TO-DO comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - it was a note to myself but something I didn't end up doing. Removed, thanks!

Base automatically changed from 3896-namespaces-via-plugin to master December 10, 2021 01:40
plugin.

Signed-off-by: Michael Nelson <minelson@vmware.com>
Signed-off-by: Michael Nelson <minelson@vmware.com>
@castelblanque castelblanque merged commit 1644250 into master Dec 10, 2021
@castelblanque castelblanque deleted the 3896-namespaces-via-plugin-2 branch December 10, 2021 08:49
Comment on lines +425 to +427
return status.Errorf(codes.PermissionDenied, "Unauthorized to %s the %s '%s' due to '%v'", verb, resource, identifier, err)
} else if errors.IsAlreadyExists(err) {
return status.Errorf(codes.AlreadyExists, "Cannot %s the %s '%s' due to '%v' as it already exists", verb, resource, identifier, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll probably want to cherry-pick this change for the kapp plugin, as I'm using a similar function.
Long-term, perhaps we could extract these util functions to a separate pkg that others can import.

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.

None yet

3 participants