Skip to content

Conversation

@Rugvip
Copy link
Member

@Rugvip Rugvip commented Aug 17, 2020

This adds an app-backend plugin, allowing for a single deployment artifact by having frontend be bundled into and served by the backend.

Config from APP_CONFIG is injected and re-injected at startup, but otherwise the frontend config comes from the actual frontend build.

This kinda makes me want to collect all plugin backends under an api/ router, so that we can have a proper 404 handler for API routes, and more straightforward load balancing. Right now you'll end up with index.html, since we want HTML5 mode routing. That's not really related to this change though, as it'd be a thing for any deployment setup.

@Rugvip Rugvip requested a review from a team as a code owner August 17, 2020 11:30
@andrewthauer
Copy link
Collaborator

This is a great idea! +1 for having backend API routes prefixed with /api (as a separate change).

@Fox32
Copy link
Contributor

Fox32 commented Aug 17, 2020

This kinda makes me want to collect all plugin backends under an api/ router

We are already doing it, right now we configure it in our k8s ingress

@dtuite
Copy link
Collaborator

dtuite commented Aug 17, 2020

@Fox32 "We are already doing it, right now we configure it in our k8s ingress"

Is there any chance this is open source? I spent some time on path based routing using k8s ingress and could not get it to work. I ended up using subdomains... backend.example.com, lighthouse.example.com and so on.

@Fox32
Copy link
Contributor

Fox32 commented Aug 17, 2020

@Fox32 "We are already doing it, right now we configure it in our k8s ingress"

Is there any chance this is open source? I spent some time on path based routing using k8s ingress and could not get it to work. I ended up using subdomains... backend.example.com, lighthouse.example.com and so on.

We have something like:

apiVersion: networking.k8s.io/v1beta1
kind: Ingress
metadata:
  annotations:
    cert-manager.io/cluster-issuer: letsencrypt
    nginx.ingress.kubernetes.io/force-ssl-redirect: "true"
    nginx.ingress.kubernetes.io/rewrite-target: /$1
  name: backstage
spec:
  rules:
    - host: $(backstage_INGRESS).$(backstage_CLUSTER_DOMAIN)
      http:
        paths:
          - backend:
              serviceName: backstage-app
              servicePort: http
            path: /(.*)
          - backend:
              serviceName: backstage-backend
              servicePort: http
            path: /api/(catalog.*)
          - backend:
              serviceName: backstage-backend
              servicePort: http
            path: /api/(auth.*)
          - backend:
              serviceName: backstage-backend
              servicePort: http
            path: /api/(identity.*)
          - backend:
              serviceName: backstage-backend
              servicePort: http
            path: /api/(proxy.*)

@dtuite
Copy link
Collaborator

dtuite commented Aug 17, 2020

@Fox32 Thank you

@timja
Copy link
Contributor

timja commented Aug 20, 2020

Huge +1 for this.

@freben
Copy link
Member

freben commented Aug 26, 2020

rebased on master just because

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of using a weird global, could we just add a requireImpl?: WhateverRequireTypeIs to RouterOptions, and have it default at runtime to the actual require? or, can we inject the actual app module or something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add a package path resolve utility to backend-common, or look into if we can solve this by changing the behaviour of __dirname in the build. Either way I don't think we want the long term solution to be to pass the actual resolve function into the plugin.

);
await fs.writeFile(path, newContent, 'utf8');
return;
} else if (content.includes('__APP_INJECTED_CONFIG_MARKER__')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gonna be that guy and ask, can you explain again why it can happen that we need to re-inject/replace an existing config?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the docker-container case you have to re-create the container if you want to change any env vars, so there's no need to support re-injection there. With this packaging method someone might be running the backend outside of docker. In that case the backend might be started with different env var values, so we need to re-inject the config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants