Skip to content

feat: use app wildcards for apps if configured #4263

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

Merged
merged 11 commits into from
Oct 5, 2022

Conversation

deansheather
Copy link
Member

@deansheather deansheather commented Sep 29, 2022

  • Adds subdomain to the WorkspaceApp struct.
  • Changes the dashboard to use subdomain app access if subdomain is true and an app hostname is set.
  • Add a tooltip explaining why the app button is greyed out if so.
  • Upgrade coder/coder terraform module to 0.5.0 (which deprecates relative_path and adds subdomain).
  • Rename relative_path to subdomain across the codebase and in the database

@deansheather deansheather marked this pull request as ready for review October 4, 2022 15:32
@deansheather deansheather requested a review from a team as a code owner October 4, 2022 15:32
@deansheather deansheather requested review from jsjoeio and f0ssel and removed request for a team October 4, 2022 15:32
Copy link
Contributor

@jsjoeio jsjoeio left a comment

Choose a reason for hiding this comment

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

Nice work!

@deansheather deansheather requested a review from kylecarbs October 4, 2022 17:10
// `coder server` or via a hostname-based dev URL. If this is set to false
// and there is no app wildcard configured on the server, the app will not
// be accessible in the UI.
RelativePath bool `json:"relative_path"`
Copy link
Member

Choose a reason for hiding this comment

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

I think we'll have to invert this on the provider to not break current deployments.

This is defaulting to false: https://github.com/coder/terraform-provider-coder/blob/main/provider/app.go#L62, which I believe would break everyone.

eg. remove relative_path and add subdomain instead

Copy link
Member Author

Choose a reason for hiding this comment

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

done!

- rename relative_path -> subdomain when referring to apps
    - migrate workspace_apps.relative_path to workspace_apps.subdomain
- upgrade coder/coder terraform module to 0.5.0
@deansheather deansheather requested a review from kylecarbs October 4, 2022 19:12
Command string `mapstructure:"command"`
Subdomain bool `mapstructure:"subdomain"`
// RelativePath is deprecated in favor of Subdomain.
RelativePath *bool `mapstructure:"relative_path"`
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this needs to be a pointer anymore

Copy link
Member Author

Choose a reason for hiding this comment

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

We prefer the deprecated value over the new value if it's set, so we have to use a pointer for this to determine if it was explicitly set or not. I will add a comment

@deansheather deansheather enabled auto-merge (squash) October 5, 2022 15:17
@deansheather deansheather merged commit 2a66395 into main Oct 5, 2022
@deansheather deansheather deleted the dean/app-relative-path branch October 5, 2022 19:23
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.

3 participants