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

pinniped 0.6.0 has backwards incompatible change #2426

Closed
absoludity opened this issue Feb 19, 2021 · 3 comments · Fixed by #2536
Closed

pinniped 0.6.0 has backwards incompatible change #2426

absoludity opened this issue Feb 19, 2021 · 3 comments · Fixed by #2536
Assignees
Labels
kind/bug An issue that reports a defect in an existing feature
Milestone

Comments

@absoludity
Copy link
Contributor

absoludity commented Feb 19, 2021

Description:

When running Kubeapps with pinniped-proxy with pinniped 0.6.0 installed on the cluster, the credential exchange fails with:

Caused by:
    err creating token exchange: "{\"apiVersion\":\"login.concierge.pinniped.dev/v1alpha1\",\"kind\":\"TokenCredentialRequest\",\"metadata\":{\"name\":\"\",\"namespace\":\"pinniped-concierge\"},\"spec\":{\"token\":\"...\",\"authenticator\":{\"apiGroup\":\"authentication.concierge.pinniped.dev\",\"kind\":\"JWTAuthenticator\",\"name\":\"jwt-authenticator\"}}}"
    ApiError: the server could not find the requested resource: NotFound (ErrorResponse { status: "Failure", message: "the server could not find the requested resource", reason: "NotFound", code: 404 })
 INFO  pinniped_proxy::service > GET https://kubernetes.default/ 500 Internal Server Error

Additional information you deem important (e.g. issue happens only occasionally):

It's not yet clear why this fails with 0.6.0 of pinniped while it works fine with 0.5.0, but we'll need to investigate further. Most likely it's related to the backwards incompatible change mentioned on the releases notes, though I have already tried re-creating the jwtauthenticator (as a cluster-scoped resource) without any change. It could be that the TokenCredentialRequest is also cluster-scoped in which case the request from pinniped-proxy will need updating.

@absoludity absoludity added the kind/bug An issue that reports a defect in an existing feature label Feb 19, 2021
@absoludity absoludity changed the title pinniped 0.6.0 has backwards incompatible chaneg pinniped 0.6.0 has backwards incompatible change Feb 25, 2021
@danlishka danlishka added this to the Q1-2 milestone Mar 2, 2021
@danlishka danlishka modified the milestones: Q1-2, 2.3 Mar 3, 2021
@antgamdia antgamdia self-assigned this Mar 8, 2021
@antgamdia
Copy link
Contributor

As discussed in the PR (#2515 (comment)), I'm putting here also the context:

We have to perform two changes:

  • Support custom API suffixes ( "my.custom.suffix" instead of"pinniped.dev").
  • Change namespace API calls to global API calls.

In both cases, we need to implement the CRD ourselves instead of using the #derive macro since runtime information is required :(

@absoludity
Copy link
Contributor Author

In both cases, we need to implement the CRD ourselves instead of using the #derive macro since runtime information is required :(

I don't know that we do. See my comments on your PR. I would just prepare a change that updates to non-namespaced (without an option) and we say pinniped >0.6.0 is now required. And for the groups, if we can just have two structs (derived for us), one for the pinniped.dev group and one for the tmc group, ... if that works, let's just do that for now and I'll look later at the dynamic group option.

@antgamdia
Copy link
Contributor

Yes, I do agree. I thought it was a bit easier and, since the 'hardcoded' approach cannot be reused by other components, I tried to pull away from this idea as much as possible. Due to the imminent rollout of 0.6.0 in our platforms, it's a good idea to -just- solve the problem and then come up with a better approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug An issue that reports a defect in an existing feature
Projects
None yet
3 participants