-
Notifications
You must be signed in to change notification settings - Fork 875
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
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"` |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
Command string `mapstructure:"command"` | ||
Subdomain bool `mapstructure:"subdomain"` | ||
// RelativePath is deprecated in favor of Subdomain. | ||
RelativePath *bool `mapstructure:"relative_path"` |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
subdomain
to the WorkspaceApp struct.subdomain
istrue
and an app hostname is set.relative_path
and addssubdomain
).relative_path
tosubdomain
across the codebase and in the database