appuio / cloud-portal

APPUiO Cloud Portal (Web Frontend)
Apache License 2.0
5 stars 0 forks source link

Fix not being able to edit Billing members #535

Open ccremer opened 1 year ago

ccremer commented 1 year ago

Summary

v0.24 of control-api has been released, which creates automatically the admin cluster role for billing entities when created. This removes the need (and assumption) that the portal should be managing both ClusterRole and ClusterRoleBinding. Now, only the ClusterRoleBinding is required to be edited.

Also fixes the redirect after saving the members -> goes back to BE details instead of Zones.

Checklist

github-actions[bot] commented 1 year ago

🚀 Preview deployment active

App URL https://portal-pr-535-appuio-control-api-preview.apps.cloudscale-lpg-2.appuio.cloud
Revision a860413e8488a61e172de85263544ddc26456c97
Helm release appuio-control-api-preview/portal-pr-535
Cluster https://api.cloudscale-lpg-2.appuio.cloud:6443

To uninstall this deployment, close or merge this PR.

bastjan commented 1 year ago

There will still be cases where a sudoer needs to create those roles from the portal GUI. We still have customers in the Odoo that might request access but can't see their BEs since they are not setup correctly.

We might add some reconciling logic for the roles but would prefer to correct the rules here until we have smth like this.

See for correct rules:

https://github.com/appuio/control-api/blob/c108373328b681637e39f1728425cd9a27046766/apiserver/billing/rbac.go#L48-L60

https://github.com/appuio/control-api/blob/c108373328b681637e39f1728425cd9a27046766/apiserver/billing/rbac.go#L73-L91

github-actions[bot] commented 1 year ago

🚀 Preview deployment active

App URL https://portal-pr-535-appuio-control-api-preview.apps.cloudscale-lpg-2.appuio.cloud
Revision ff1cccca61e191b19757d47fe0d25aaef78c0e5c
Helm release appuio-control-api-preview/portal-pr-535
Cluster https://api.cloudscale-lpg-2.appuio.cloud:6443

To uninstall this deployment, close or merge this PR.

github-actions[bot] commented 1 year ago

🚀 Preview deployment active

App URL https://portal-pr-535-appuio-control-api-preview.apps.cloudscale-lpg-2.appuio.cloud
Revision 06d0036f503ba35d20efb52a4c933f5a37ad403a
Helm release appuio-control-api-preview/portal-pr-535
Cluster https://api.cloudscale-lpg-2.appuio.cloud:6443

To uninstall this deployment, close or merge this PR.

github-actions[bot] commented 1 year ago

🚀 Preview deployment active

App URL https://portal-pr-535-appuio-control-api-preview.apps.cloudscale-lpg-2.appuio.cloud
Revision a3c0029a997b5016bd770e5231dc600f46d9ce79
Helm release appuio-control-api-preview/portal-pr-535
Cluster https://api.cloudscale-lpg-2.appuio.cloud:6443

To uninstall this deployment, close or merge this PR.

ccremer commented 1 year ago

@corvus-ch @bastjan I don't quite understand what the holdup is for this fix. Right now (also in Prod), when users create a billing address and then try to edit members (billingentities/be-xxxx/members), they get presented with an error and the glitchtip reporter dialog. This PR primarily addresses this issue so the reporter doesn't show up anymore.

bastjan commented 1 year ago

As i said: there is no reconciliation and no guarantee that the admin role is created if an existing, wrongly setup BE is enabled/fixed. We still need the admin role creation for VSHN support team members.

I'd strongly prefer to update the role creation code with the rules i linked.

ccremer commented 1 year ago

Ok, leaving this for now. Let me know if you prefer to disable this view temporarily, so that users can't click/visit the BE members list view.

github-actions[bot] commented 1 year ago

🚀 Preview deployment active

App URL https://portal-pr-535-appuio-control-api-preview.apps.cloudscale-lpg-2.appuio.cloud
Revision eae00b7beed53fc08f0f297ab26e958f7e68a383
Helm release appuio-control-api-preview/portal-pr-535
Cluster https://api.cloudscale-lpg-2.appuio.cloud:6443

To uninstall this deployment, close or merge this PR.

bastjan commented 1 year ago

I might misunderstand the issue.

Why not just create the roles as defined here? https://github.com/appuio/cloud-portal/pull/535#issuecomment-1511016363

corvus-ch commented 1 year ago

As far as I understand, this issue does not surface when users create their BillingEntity on their own. Only when they get created in the ERP. So, this is not an issue to expect within the next days. Instead of wasting time with some workarounds (which we might have to remove again later) in the frontend, I would prefer to fix this for good in the API.

HappyTetrahedron commented 1 year ago

As far as I have just tested, this issue does in fact surface for user-created BillingEntities as well.

I just created a billingentity through the portal (using a new account created through social login), and subsequently when I click on "edit members", the issue described above pops up (error message + glitchtip popup). So it is not as uncommon as we initially thought.

github-actions[bot] commented 1 year ago

🚀 Preview deployment active

App URL https://portal-pr-535-appuio-control-api-preview.apps.cloudscale-lpg-2.appuio.cloud
Revision 60c87bab9048887bbdcf42dd6d17e098c588f7fb
Helm release appuio-control-api-preview/portal-pr-535
Cluster https://api.cloudscale-lpg-2.appuio.cloud:6443

To uninstall this deployment, close or merge this PR.