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

🔄 Update Control Panel Process for App Creation Post-Migration #2633

Closed
6 tasks done
Tracked by #1700
julialawrence opened this issue Dec 4, 2023 · 8 comments · Fixed by ministryofjustice/analytics-platform-control-panel#1228
Assignees
Labels
data-platform-apps-and-tools This issue is owned by Data Platform Apps and Tools enhancement enhancing an existing feature

Comments

@julialawrence
Copy link
Contributor

julialawrence commented Dec 4, 2023

User Story

As an Analytical Platform Developer
I would like to reenable the new app functionality in Control Panel currently disabled due to recently completed app migration.
Now that the migration has completed, our customers can resume self-servicing their new app requests.

Value / Purpose

Reenabling this functionality post-migration is critical to reenable app creation process for our customers, ensuring smooth operations and user satisfaction on our Analytical Platform.
This has been a frequent customer ask

Useful Contacts

@julialawrence @ymao2

User Types

AP Customers

Hypothesis

If we reenable the new app functionality, customers will be able to take advantage

Proposal

  • Agree the form being made public or continuing to be admin-only
  • Deploy a control panel patch which reenables the new app feature, which handles role creation, datasource creation and planting of secrets.
  • Review create app form to see if it still relevant and modify accordingly

Additional Information

To simplify the process both for the customer and AP devs, the expectation is that the new app repo will be created according to instructions written up for #2632.
New app repos MUST be created in the ministryofjustice github org

Definition of Done

  • Agree whether create app for should remain admin-only or be made public
  • Review create app form for accuracy and amend if not.
  • The app creation functionality Control Panel has been successfully reenabled.
  • Tests to ensure the functionality works as expected post-migration have been conducted and passed.
  • All changes reviewed.
  • Documentation updated
@julialawrence julialawrence added the enhancement enhancing an existing feature label Dec 4, 2023
@julialawrence julialawrence added data-platform-apps-and-tools This issue is owned by Data Platform Apps and Tools [📝 ] Register a new app (Epic #1700) labels Dec 4, 2023
@julialawrence julialawrence changed the title 🔄 Reenabling Python-Based Control Panel for App Creation Post-Migration 🔄 Reenabling Control Panel for App Creation Post-Migration Dec 4, 2023
@github-actions github-actions bot mentioned this issue Dec 4, 2023
9 tasks
@michaeljcollinsuk
Copy link
Contributor

@ymao2 can you confirm that the "Register an app" functionality has not need disabled, but that only an admin/superuser can do this?

If so, are we going to make the "register an app" form accessible to all users to complete?

@julialawrence julialawrence changed the title 🔄 Reenabling Control Panel for App Creation Post-Migration 🔄 Review Control Panel Process for App Creation Post-Migration Dec 5, 2023
@michaeljcollinsuk
Copy link
Contributor

michaeljcollinsuk commented Dec 6, 2023

Proposed fields for new "simplified" version of the register an app form if we decide to make it publicly accessible:

  • Github repository <-- Keep but modify
    • Make this standard textfield, removing the choice of organisation and preload options
    • Add validation to check that the entered url begins with https://github.com/ministryofjustice/
  • Available Deployment Environments ??????
    • Could remove this option and then require the Auth0 clients to be created from the "Manage App" page?
    • Q: are Environments a prerequisite to this step, so will always have been created? May not be important
    • Potential bug - currently if no envs are selected, then there is no way to later them via the "manage app" page. This may need to be added? SOLVED this was because of querystring params in the repo url,it fails to return the envs from github.
  • Datasource **<-- Keep as is **
  • Connections
    • If we remove the deployment env options, then remove this
  • IP allowlists
    • Same as above - if we remove the deployment env options, then remove this

@michaeljcollinsuk michaeljcollinsuk moved this from 🧐 To Do to 💨 In Progress in Analytical Platform Dec 7, 2023
@michaeljcollinsuk
Copy link
Contributor

michaeljcollinsuk commented Dec 7, 2023

Decided to make a simplified form publicly accessible.

Fields:

  • Github repository
    • Simplify to a single field
    • Amend validation to ensure the repo is in the MOJ org
  • Datasource
    • Keep as is

Changes required to the logic

  • When the form is submitted:
    • No auth0 clients will be created. These will be created via button on the "Manage app" page.
    • This means removing the post-signal hook that sends a task to create auth settings

Questions/TBC:

  1. Should a field be added to the app management page to allow the user to enter the app namespace? We need to know the namespace to use it when updating the roles Trust Relationship to allow Cloud Platform to assume the role
  2. Alternative would be to strictly enforce a naming convention for the namespace, then we can assume it based on the app/repo name e.g. data-platform-app-repo-name
  3. Or drop the AWS role creation from control panel entirely?

1 - not neccessary if we use naming convention
2 - yes use naming convention
3 - no, still create the role, and update the trust relationship at the same time

@michaeljcollinsuk
Copy link
Contributor

AWS Role notes:

  • Role is shared for both prod/dev environments
  • In future, there may be a change to create roles for each env - this would be a new feature, not part of this work
  • Need to update the Control Panel so that the trust relationship is amended to allow Cloud Platform to assume the role. We will use a naming convention for this

@michaeljcollinsuk michaeljcollinsuk moved this from 💨 In Progress to 👀 In Review in Analytical Platform Jan 15, 2024
@michaeljcollinsuk michaeljcollinsuk changed the title 🔄 Review Control Panel Process for App Creation Post-Migration 🔄 Update Control Panel Process for App Creation Post-Migration Jan 15, 2024
@michaeljcollinsuk michaeljcollinsuk changed the title 🔄 Update Control Panel Process for App Creation Post-Migration 🔄 Review Control Panel Process for App Creation Post-Migration Jan 15, 2024
@michaeljcollinsuk michaeljcollinsuk changed the title 🔄 Review Control Panel Process for App Creation Post-Migration 🔄 Update Control Panel Process for App Creation Post-Migration Jan 15, 2024
@julialawrence julialawrence moved this from 👀 In Review to ✋ Blocked in Analytical Platform Jan 19, 2024
@michaeljcollinsuk
Copy link
Contributor

michaeljcollinsuk commented Jan 19, 2024

As part of resolving a support issue, a couple of things became apparent:

  1. The "Create auth0 client" button only appears to superusers - see permission check and corresponding criteria for this permission
  2. After creating an auth0 client, the app needs to be redeployed - in the context of registering an app, this is probably ok as you would expect the app owner to still be working on the application code? We will just need to instruct users to create a PR to deploy the dev site, and then merge to deploy the prod site.

Actions:

  1. Update the code to make this button available to app owners. Alternatively, create these by default when registering the app. One downside to this, is it may create unneeded clients (although not if the user had already deleting there dev environment)
  2. Document that a deployment is needed after creating the auth0 client. Perhaps this is where to add instructions about adding a Dockerfile to the repo?

@julialawrence
Copy link
Contributor Author

If we don't get ublocked by end of sprint, these go to the backlog and another story raised to test with DMET. Testing is 1 meeting for 2 hours.

@michaeljcollinsuk
Copy link
Contributor

michaeljcollinsuk commented Jan 30, 2024

Closed when linked PR was merged, but there is still additional work to complete so reopening

@michaeljcollinsuk michaeljcollinsuk moved this from ✋ Blocked to 👀 In Review in Analytical Platform Feb 1, 2024
@michaeljcollinsuk
Copy link
Contributor

Functionality has been reimplement in PR's:
ministryofjustice/analytics-platform-control-panel#1228
ministryofjustice/analytics-platform-control-panel#1240

The changes are being deployed to dev for testing. Closing this ticket as the final run through, and deployment to production is being tracked in #2636

@michaeljcollinsuk michaeljcollinsuk moved this from 👀 In Review to 🎉 Done in Analytical Platform Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data-platform-apps-and-tools This issue is owned by Data Platform Apps and Tools enhancement enhancing an existing feature
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants