Skip to content

feat: allow configuring display apps from template #9100

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 7 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
pr comments
  • Loading branch information
sreya committed Aug 29, 2023
commit 9be2234b02ce72dec1f05766ca444e66b8231bc5
2 changes: 0 additions & 2 deletions coderd/database/modelmethods.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import (
"github.com/coder/coder/v2/coderd/rbac"
)

var DefaultDisplayApps = []DisplayApp{DisplayAppVscode, DisplayAppWebTerminal, DisplayAppSSHHelper, DisplayAppPortForwardingHelper}

type WorkspaceStatus string

const (
Expand Down
2 changes: 1 addition & 1 deletion coderd/provisionerdserver/provisionerdserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1528,7 +1528,7 @@ func TestInsertWorkspaceResource(t *testing.T) {
}, agent.DisplayApps)
})

t.Run("AllDefaultApps", func(t *testing.T) {
t.Run("AllDisplayApps", func(t *testing.T) {
t.Parallel()
db := dbfake.New()
job := uuid.New()
Expand Down
13 changes: 4 additions & 9 deletions provisioner/terraform/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
stringutil "github.com/coder/coder/v2/coderd/util/strings"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/provisioner"
"github.com/coder/coder/v2/provisionersdk"
"github.com/coder/coder/v2/provisionersdk/proto"
)

Expand Down Expand Up @@ -192,16 +193,10 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string) (*State, error

// If a user doesn't specify 'display_apps' then they default
// into all apps except VSCode Insiders.
displayApps := proto.DisplayApps{
Vscode: true,
VscodeInsiders: false,
WebTerminal: true,
PortForwardingHelper: true,
SshHelper: true,
}
displayApps := provisionersdk.DefaultDisplayApps()

if len(attrs.DisplayApps) != 0 {
displayApps = proto.DisplayApps{
displayApps = &proto.DisplayApps{
Vscode: attrs.DisplayApps[0].VSCode,
VscodeInsiders: attrs.DisplayApps[0].VSCodeInsiders,
WebTerminal: attrs.DisplayApps[0].WebTerminal,
Expand All @@ -226,7 +221,7 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string) (*State, error
ShutdownScript: attrs.ShutdownScript,
ShutdownScriptTimeoutSeconds: attrs.ShutdownScriptTimeoutSeconds,
Metadata: metadata,
DisplayApps: &displayApps,
DisplayApps: displayApps,
}
switch attrs.Auth {
case "token":
Expand Down
19 changes: 14 additions & 5 deletions provisionersdk/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
_ "embed"
"fmt"
"strings"

"github.com/coder/coder/v2/provisionersdk/proto"
)

var (
Expand Down Expand Up @@ -35,11 +37,6 @@ var (
"arm64": darwinScript,
},
}

// AllDisplayApps indicates that a user has not specified a 'display_apps' block in their template.
// There is no way to distinguish between a nil array (unset) and an empty array in protobuf and an
// empty array already signifies that all display apps should be disabled.
AllDisplayApps = "*"
)

// AgentScriptEnv returns a key-pair of scripts that are consumed
Expand All @@ -55,3 +52,15 @@ func AgentScriptEnv() map[string]string {
}
return env
}

// DefaultDisplayApps returns the default display applications to enable
// if none are specified in a template.
func DefaultDisplayApps() *proto.DisplayApps {
return &proto.DisplayApps{
Vscode: true,
VscodeInsiders: false,
WebTerminal: true,
PortForwardingHelper: true,
SshHelper: true,
}
}