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

bug: APIs bound through Permission Claims not available when using identityHash #1845

Closed
johnmcollier opened this issue Aug 26, 2022 · 12 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.

Comments

@johnmcollier
Copy link

johnmcollier commented Aug 26, 2022

Describe the bug

When specifying another APIExport's identityHash in a permission claim in an APIExport / APIBinding, the APIBinding will create successfully:

E.g.:

apiVersion: apis.kcp.dev/v1alpha1
kind: APIBinding
metadata:
  name: has-binding
spec:
  reference:
    workspace:
      path: root:has
      exportName: has
  acceptedPermissionClaims:
      - group: ""
        resource: "secrets"
        verbs: ["get", "list", "watch"]
      - group: ""
        resource: "configmaps"
        verbs: ["get", "list", "watch"]
      - identityHash: 5d8e0fa5f9a10dbff668aad2cd63777e061c837e3f0a2f1adda15f7acae9fd53
        group: "appstudio.redhat.com"
        resource: "environments"
      - identityHash: 5d8e0fa5f9a10dbff668aad2cd63777e061c837e3f0a2f1adda15f7acae9fd53
        group: "appstudio.redhat.com"
        resource: "applicationsnapshotenvironmentbindings"

with the following status conditions:

  conditions:
  - lastTransitionTime: "2022-08-26T18:24:05Z"
    status: "True"
    type: Ready
  - lastTransitionTime: "2022-08-26T18:24:05Z"
    status: "True"
    type: APIExportValid
  - lastTransitionTime: "2022-08-26T18:24:05Z"
    status: "True"
    type: BindingUpToDate
  - lastTransitionTime: "2022-08-26T18:24:05Z"
    status: "True"
    type: InitialBindingCompleted
  - lastTransitionTime: "2022-08-26T18:24:05Z"
    status: "True"
    type: PermissionClaimsApplied
  - lastTransitionTime: "2022-08-26T18:24:05Z"
    status: "True"
    type: PermissionClaimsValid

But if I run kubectl get environments the command fails, saying error: the server doesn't have a resource type "environments"

Steps To Reproduce

I've created a reporducer here: https://github.com/johnmcollier/kcp-permissionclaims-reproducer

Expected Behaviour

The APIs specified in the permission claims are available.

Additional Context

No response

@johnmcollier johnmcollier added the kind/bug Categorizes issue or PR as related to a bug. label Aug 26, 2022
@csams csams added this to kcp Aug 26, 2022
@csams csams moved this to New in kcp Aug 26, 2022
@ncdc
Copy link
Member

ncdc commented Aug 26, 2022

/assign

@ncdc
Copy link
Member

ncdc commented Aug 27, 2022

@johnmcollier please try adding another APIBinding in your workspace, to the gitops APIExport.

/priority awaiting-more-evidence

@openshift-ci openshift-ci bot added the priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. label Aug 27, 2022
@johnmcollier
Copy link
Author

johnmcollier commented Aug 29, 2022

@ncdc Yes, if I add another APIBinding the APIs in the gitops APIExport become available. But I thought the permission claims meant that wasn't needed?

// permissionClaims make resources available in APIExport's virtual workspace that are not part
// of the actual APIExport resources.
//
// PermissionClaims are optional and should be the least access necessary to complete the functions
// that the service provider needs. Access is asked for on a GroupResource + identity basis.
//
// PermissionClaims must be accepted by the user's explicit acknowledgement. Hence, when claims
// change, the respecting objects are not visible immediately.
//
// PermissionClaims overlapping with the APIExport resources are ignored.
//
// +optional
// +listType=map
// +listMapKey=group
// +listMapKey=resource
PermissionClaims []PermissionClaim `json:"permissionClaims,omitempty"`

@stevekuznetsov
Copy link
Contributor

Is this #1183 ?

@ncdc
Copy link
Member

ncdc commented Aug 29, 2022

@stevekuznetsov nope, different

@ncdc
Copy link
Member

ncdc commented Aug 29, 2022

@johnmcollier both bindings are required

@ncdc
Copy link
Member

ncdc commented Aug 29, 2022

This is not a bug

@ncdc
Copy link
Member

ncdc commented Aug 29, 2022

Closing

@ncdc ncdc closed this as completed Aug 29, 2022
Repository owner moved this from New to Done in kcp Aug 29, 2022
@johnmcollier
Copy link
Author

johnmcollier commented Aug 29, 2022

Then should the description for PermissionClaim be updated?

Since the description:

// permissionClaims make resources available in APIExport's virtual workspace that are not part 
// of the actual APIExport resources. `

makes it seem like the second APIBinding is not needed.

@ncdc
Copy link
Member

ncdc commented Aug 29, 2022

That is only relevant for API providers accessing their APIExport's virtual workspace.

@johnmcollier
Copy link
Author

johnmcollier commented Aug 29, 2022

@ncdc So does that mean a controller running in my APIExport's virtual workspace should have the API(s) available there without a separate APIBinding?

If so, my controller still fails on start up saying those APIs aren't available in the virtual workspace.

@ncdc
Copy link
Member

ncdc commented Aug 29, 2022

Now that is #1183 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.
Projects
Status: Done
Development

No branches or pull requests

3 participants