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.

Sign up for GitHub

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
Merged

feat: use app wildcards for apps if configured #4263

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
dbb2e0e
feat: use app wildcards for apps if configured
deansheather Sep 29, 2022
c8d2da9
fixup! feat: use app wildcards for apps if configured
deansheather Sep 29, 2022
5bd46a9
feat: working tooltips on disabled app buttons
deansheather Oct 4, 2022
35596a7
feat: default apps relative_path to true
deansheather Oct 4, 2022
6a8339e
feat: relative_path -> subdomain
deansheather Oct 4, 2022
4d75f5c
fixup! feat: relative_path -> subdomain
deansheather Oct 4, 2022
e51f69d
fixup! feat: relative_path -> subdomain
deansheather Oct 4, 2022
ea0a38f
fixup! feat: relative_path -> subdomain
deansheather Oct 4, 2022
4f6ab19
fixup! feat: relative_path -> subdomain
deansheather Oct 5, 2022
e4e8663
Merge branch 'main' into dean/app-relative-path
deansheather Oct 5, 2022
26b5e57
empty commit to make github actions work
deansheather Oct 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions coderd/workspaceagents.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,10 +489,11 @@ func convertApps(dbApps []database.WorkspaceApp) []codersdk.WorkspaceApp {
apps := make([]codersdk.WorkspaceApp, 0)
for _, dbApp := range dbApps {
apps = append(apps, codersdk.WorkspaceApp{
ID: dbApp.ID,
Name: dbApp.Name,
Command: dbApp.Command.String,
Icon: dbApp.Icon,
ID: dbApp.ID,
Name: dbApp.Name,
Command: dbApp.Command.String,
Icon: dbApp.Icon,
RelativePath: dbApp.RelativePath,
Healthcheck: codersdk.Healthcheck{
URL: dbApp.HealthcheckUrl,
Interval: dbApp.HealthcheckInterval,
Expand Down
5 changes: 5 additions & 0 deletions codersdk/workspaceapps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ type WorkspaceApp struct {
// Icon is a relative path or external URL that specifies
// an icon to be displayed in the dashboard.
Icon string `json:"icon,omitempty"`
// RelativePath denotes whether the app should be accessed via a path on the
// `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!

// Healthcheck specifies the configuration for checking app health.
Healthcheck Healthcheck `json:"healthcheck"`
Health WorkspaceAppHealth `json:"health"`
Expand Down
23 changes: 16 additions & 7 deletions provisioner/terraform/resources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@ type agentAttributes struct {

// A mapping of attributes on the "coder_app" resource.
type agentAppAttributes struct {
AgentID string `mapstructure:"agent_id"`
Name string `mapstructure:"name"`
Icon string `mapstructure:"icon"`
URL string `mapstructure:"url"`
Command string `mapstructure:"command"`
RelativePath bool `mapstructure:"relative_path"`
AgentID string `mapstructure:"agent_id"`
Name string `mapstructure:"name"`
Icon string `mapstructure:"icon"`
URL string `mapstructure:"url"`
Command string `mapstructure:"command"`
// RelativePath is optional, but it defaults to true so we have to make it
// a pointer to be backwards compatible.
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

Healthcheck []appHealthcheckAttributes `mapstructure:"healthcheck"`
}

Expand Down Expand Up @@ -238,6 +240,13 @@ func ConvertResources(module *tfjson.StateModule, rawGraph string) ([]*proto.Res
Threshold: attrs.Healthcheck[0].Threshold,
}
}

// Default attrs.RelativePath to true if unspecified in Terraform.
relativePath := true
if attrs.RelativePath != nil {
relativePath = *attrs.RelativePath
}

for _, agents := range resourceAgents {
for _, agent := range agents {
// Find agents with the matching ID and associate them!
Expand All @@ -249,7 +258,7 @@ func ConvertResources(module *tfjson.StateModule, rawGraph string) ([]*proto.Res
Command: attrs.Command,
Url: attrs.URL,
Icon: attrs.Icon,
RelativePath: attrs.RelativePath,
RelativePath: relativePath,
Healthcheck: healthcheck,
})
}
Expand Down
28 changes: 19 additions & 9 deletions provisioner/terraform/resources_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,26 @@ func TestConvertResources(t *testing.T) {
Name: "dev1",
OperatingSystem: "linux",
Architecture: "amd64",
Apps: []*proto.App{{
Name: "app1",
}, {
Name: "app2",
Healthcheck: &proto.Healthcheck{
Url: "http://localhost:13337/healthz",
Interval: 5,
Threshold: 6,
Apps: []*proto.App{
{
Name: "app1",
// RelativePath defaults to true if unspecified.
RelativePath: true,
},
}},
{
Name: "app2",
RelativePath: false,
Healthcheck: &proto.Healthcheck{
Url: "http://localhost:13337/healthz",
Interval: 5,
Threshold: 6,
},
},
{
Name: "app3",
RelativePath: true,
},
},
Auth: &proto.Agent_Token{},
}},
}},
Expand Down
13 changes: 12 additions & 1 deletion provisioner/terraform/testdata/multiple-apps/multiple-apps.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,30 @@ resource "coder_agent" "dev1" {
arch = "amd64"
}

# app1 is for testing relative_path default.
resource "coder_app" "app1" {
agent_id = coder_agent.dev1.id
# relative_path should default to true.
# relative_path = true
}

# app2 tests that relative_path can be false, and that healthchecks work.
resource "coder_app" "app2" {
agent_id = coder_agent.dev1.id
agent_id = coder_agent.dev1.id
relative_path = false
healthcheck {
url = "http://localhost:13337/healthz"
interval = 5
threshold = 6
}
}

# app3 tests that relative_path can explicitly be true.
resource "coder_app" "app3" {
agent_id = coder_agent.dev1.id
relative_path = true
}

resource "null_resource" "dev" {
depends_on = [
coder_agent.dev1
Expand Down
3 changes: 3 additions & 0 deletions provisioner/terraform/testdata/multiple-apps/multiple-apps.tfplan.dot
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

120 changes: 81 additions & 39 deletions provisioner/terraform/testdata/multiple-apps/multiple-apps.tfplan.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions provisioner/terraform/testdata/multiple-apps/multiple-apps.tfstate.dot
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading