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

Add Connect Enterprise entry point #40477

Closed
wants to merge 7 commits into from
Closed

Conversation

gzdunek
Copy link
Contributor

@gzdunek gzdunek commented Apr 11, 2024

The first step of #17706.

e counterpart https://github.com/gravitational/teleport.e/pull/3946

A few notes:

  • In the config files I use both absolute and relative paths.
    • The absolute ones are used for the source code, icons, scripts, etc, since we generally want both OSS and Enterprise use the same files (and Enterprise can override them).
    • The relative paths are used for input files/output paths - we want to build files stay local for both app versions.
  • Electron version and app dependencies in externalizeDepsPlugin can no longer be resolved automatically, because they are read from the direct package.json so teleterm.e package.json doesn't have them.
    • Instead they are always read from the OSS package.json.
  • The scenario when one app depends on the another doesn't work well in electron-builder. When I added @gravitational/teleterm as a dependency to @gravitational/teleterm.e the app has grown 3x times because it just copied @gravitational/teleterm to the app package. I believe it happens somwhere in app-builder, but I didn't find a way to pass excludedDependencies there.
    • Instead, I'm just ignoring '!**/node_modules/@gravitational/teleterm/**' in files. Seems to work fine, both OSS and Enterprise DMGs are now ~128.5 MB.

@ravicious you mentioned in your WIP PR this:

  • teleterm.e builder-debug.yml doesn’t include '!build_resources{,/**/*}'
    How important it is? I quickly tried excluding it with!build_resources and it resulted in 3 KB size difference, so I'm not sure if I should dig there.

@gzdunek gzdunek added teleport-connect Issues related to Teleport Connect. no-changelog Indicates that a PR does not require a changelog entry labels Apr 11, 2024
@gzdunek gzdunek marked this pull request as ready for review April 11, 2024 16:00
@gzdunek gzdunek requested review from ravicious and ryanclark April 11, 2024 16:00
@github-actions github-actions bot requested review from avatus and rudream April 11, 2024 16:00
@gzdunek gzdunek removed request for avatus and rudream April 11, 2024 16:00
// from Connect OSS package.json when it is run in the enterprise app context
// (it simply reads the direct package.json content).
// We have to provide them manually.
include: Object.keys(packageJson.dependencies),
Copy link
Member

Choose a reason for hiding this comment

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

What if teleterm.e needs to depend on deps that teleterm doesn't depend on? teleport.e already depends on extra deps, so sooner or later teleterm.e will have its own deps as well.

@ravicious
Copy link
Member

ravicious commented Apr 12, 2024

Overall this looks okay, barring the question of teleterm.e specific deps. But I just raised a question on Slack if we even need teleterm.e in the first place.

@gzdunek
Copy link
Contributor Author

gzdunek commented Apr 12, 2024

We will move access requests to OSS and get rid of teleterm.e.

@gzdunek gzdunek closed this Apr 12, 2024
@gzdunek gzdunek deleted the gzdunek/connect-e-entrypoint branch July 22, 2024 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Indicates that a PR does not require a changelog entry size/sm teleport-connect Issues related to Teleport Connect. ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants