overhangio / tutor-mfe

This plugin makes it possible to easily add micro frontend (MFE) applications on top of an Open edX platform that runs with Tutor.
GNU Affero General Public License v3.0
22 stars 95 forks source link

Authn MFE is broken in nightly #122

Closed kdmccormick closed 1 year ago

kdmccormick commented 1 year ago

Steps to reproduce

Notes

ghassanmas commented 1 year ago

Hey, probably it would be useful to provide the console output, i,e are there specif API calls that are failing? And with nightly it means it suppose to run frontned-app-authn on master branch correct? both edx-platform and the mfe

kdmccormick commented 1 year ago
TypeError: secondaryProviders is undefined

Call Stack
 renderThirdPartyAuth
  src/login/LoginPage.jsx:213:38
 renderForm
  src/login/LoginPage.jsx:337:20
 render
  src/login/LoginPage.jsx:379:17
 finishClassComponent
  node_modules/react-dom/cjs/react-dom.development.js:17160:31
 updateClassComponent
  node_modules/react-dom/cjs/react-dom.development.js:17110:44
 beginWork
  node_modules/react-dom/cjs/react-dom.development.js:18620:16
 callCallback
  node_modules/react-dom/cjs/react-dom.development.js:188:14
 invokeGuardedCallbackDev
  node_modules/react-dom/cjs/react-dom.development.js:237:16
 invokeGuardedCallback
  node_modules/react-dom/cjs/react-dom.development.js:292:31
 beginWork$1
  node_modules/react-dom/cjs/react-dom.development.js:23203:28
ghassanmas commented 1 year ago

One guess it might be due to frontend-platform, since it handles the init part, could you try running the authn while dumping the frontend-platform version to 4.1.0

brian-smith-tcril commented 1 year ago

did a little testing on devstack to try to narrow down the issue

in a devstack venv

make dev.up.cms+studio

outside of the devstack venv cloned latest frontend-app-authn, in the frontend-app-authn dir

nvm use
npm install
npm start

localhost:1999 is working as expected

ghassanmas commented 1 year ago

Humm.. then it can be probably version node conflict, I think tutor tries to build using node 16 while the authin is already updated to 18 openedx/frontend-app-authn/pull/781 to test this one can just check insdie tutor-mfe contianer if assest exists in /openedx/dist/authin

brian-smith-tcril commented 1 year ago

tried

rm -rf node_modules/
nvm use 16
npm install
npm start

still seems to be working

ghassanmas commented 1 year ago

Can you try running it using dynamic config enabled I think this one main difference between devstack and tutor. You would need to swt ENV variable to the minimum needed and then serve the other by setting dynamic config api

ghassanmas commented 1 year ago

[update] I don't know if that's help. but I tried to run/build the authn mfe master while using tutor palm (using palm.master instead of master and it worked with no problem. In any case this probably mean the issue irrelevant to frontend-platform version

ghassanmas commented 1 year ago

I was able to reproduce this issue, and to resolved in hacky way where I cloned maste of authn and then mounted the dist to mfe the container. When I noticed is that the mfe image doesn't have nightly suffix, as opposite the openedx image. So I guess it was using the olive image for nightly.

(edx-env) ➜  tutor-nightly tutor config printvalue DOCKER_IMAGE_OPENEDX
docker.io/overhangio/openedx:15.3.4-nightly
(edx-env) ➜  tutor-nightly tutor config printvalue MFE_DOCKER_IMAGE
docker.io/overhangio/openedx-mfe:15.0.5

So may be running tutor images build mfe suppose to resolve?.... it's building now for me, so I can update when it's done

kdmccormick commented 1 year ago

@arbrandes will you be able to take a look? Is there anything we can do to help?

arbrandes commented 1 year ago

I can take a look over the next couple of days, yes!

brian-smith-tcril commented 1 year ago

I've been doing a bit of digging into this, when running tutor dev start I noticed

  Your models in app(s): 'third_party_auth' have changes that are not yet reflected in a migration, and so won't be applied.
  Run 'manage.py makemigrations' to make new migrations, and then re-run 'manage.py migrate' to apply them.

when breaking into the errors using a browser debugger I found issues with how we were setting thirdPartyAuthContext, it seems that has changed recently https://github.com/openedx/frontend-app-authn/blame/master/src/common-components/data/service.js#L23

I'm still looking into this, I'll make sure to share updates as I discover more.

brian-smith-tcril commented 1 year ago

Update: running tutor images build authn-dev fixed this for me.

Successfully built b058f87eab3d
Successfully tagged overhangio/openedx-authn-dev:15.0.5

compared to the current image on dockerhub (15.0.2 - 5 months ago) https://hub.docker.com/r/overhangio/authn-dev/tags

brian-smith-tcril commented 1 year ago

@regisb do you have any insights into this? It seems the required frontend-app-authn change was https://github.com/openedx/frontend-app-authn/commit/62e8f75b96994d9a281f6a7a217ae7632665d9af

I think my main concern is that running tutor dev launch on nightly doesn't guarantee the latest MFE code is pulled.

regisb commented 1 year ago

We do not push nightly images to dockerhub for MFEs, so users need to re-build them locally. @brian-smith-tcril do I understand correctly that re-building fixes the issue for you?

brian-smith-tcril commented 1 year ago

do I understand correctly that re-building fixes the issue for you?

Yes

running tutor images build authn-dev fixed this for me.

I didn't test tutor local but I assume tutor images build mfe would work for that

bmtcril commented 1 year ago

It worked on tutor local for myself and some other folks.

Is there any indication of when someone would need to rebuild the MFE image locally, or is it just "something broke, try rebuilding the image"? What causes the MFE image to be pushed to dockerhub now?

regisb commented 1 year ago

The docs state that nightly images can be pulled from dockerhub: https://docs.tutor.overhang.io/tutorials/nightly.html#upgrading-to-the-latest-version-of-open-edx

But this is factually incorrect. Only the "openedx" image is pushed to dockerhub: https://hub.docker.com/r/overhangio/openedx/tags?page=1&name=nightly CI does not push nightly images for plugins.

To offer a better user experience in nightly, we should also push nightly images for plugins. But it is unlikely that non-official images will go to the trouble of setting up a CI to push nightly images (with a different tag) to their registries.

So, I'm not sure what we should be doing here. Should we just rebuild all images automatically during launch on the nightly branch? Of course, at the very least we should fix the docs.

arbrandes commented 1 year ago

Should we just rebuild all images automatically during launch on the nightly branch?

If the images don't already exist locally, I think this is perfectly reasonable.

regisb commented 1 year ago

OK I've made the first step to resolve this issue. As of today, all Docker image tags from the nightly branches will be suffixed with "-nightly". This will prevent users from pulling Olive (or the current release) images when they really want to run nightly.

Next step will consist in running CI and push images periodically. Either that or force a re-build of all images on launch. I must admit that the thought of re-building all images from scratch every night (and fixing the issues every damn time) gives me cold sweats...

regisb commented 1 year ago

I think we really don't want to re-build the mfe nightly images every day, so I'll close this. Feel free to reopen if you think differently.