Skip to content

feat: use app wildcards for apps if configured#4263

Merged
deansheather merged 11 commits into
mainfrom
dean/app-relative-path
Oct 5, 2022
Merged

feat: use app wildcards for apps if configured#4263
deansheather merged 11 commits into
mainfrom
dean/app-relative-path

Conversation

@deansheather

@deansheather deansheather commented Sep 29, 2022

Copy link
Copy Markdown
Member
  • 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 f0ssel and jsjoeio and removed request for a team October 4, 2022 15:32

@jsjoeio jsjoeio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work!

Comment thread provisioner/terraform/testdata/multiple-apps/multiple-apps.tf Outdated
@deansheather deansheather requested a review from kylecarbs October 4, 2022 17:10
Comment thread codersdk/workspaceapps.go Outdated
// `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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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