Skip to content

chore: Refactor Enterprise code to layer on top of AGPL #4034

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 25 commits into from
Sep 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
fd05de9
chore: Refactor Enterprise code to layer on top of AGPL
kylecarbs Sep 13, 2022
04c5d1d
Merge branch 'main' into entfactor
kylecarbs Sep 13, 2022
aad7be4
Fix Garrett's comments
kylecarbs Sep 13, 2022
d0526ed
Merge branch 'main' into entfactor
kylecarbs Sep 15, 2022
27f53aa
Add pointer.Handle to atomically obtain references
kylecarbs Sep 15, 2022
ed5b96d
Remove entitlements API from AGPL coderd
kylecarbs Sep 15, 2022
be96fff
Remove AGPL Coder entitlements endpoint test
kylecarbs Sep 15, 2022
b4cbd6c
Fix warnings output
kylecarbs Sep 15, 2022
ad3cb79
Merge branch 'main' into entfactor
kylecarbs Sep 15, 2022
1964a64
Add command-line flag to toggle audit logging
kylecarbs Sep 15, 2022
0e88f51
Fix hasLicense being set
kylecarbs Sep 15, 2022
586b3e8
Remove features interface
kylecarbs Sep 15, 2022
1bb6d0f
Fix audit logging default
kylecarbs Sep 15, 2022
8c603f6
Add bash as a dependency
kylecarbs Sep 16, 2022
2dcf1f8
Add comment
kylecarbs Sep 16, 2022
2cfcfac
Merge branch 'main' into entfactor
kylecarbs Sep 18, 2022
5215fe9
Merge branch 'main' into entfactor
kylecarbs Sep 19, 2022
1955ae3
Add tests for resync and pubsub, and add back previous exp backoff retry
kylecarbs Sep 19, 2022
e01a6ce
Separate authz code again
kylecarbs Sep 19, 2022
b0a3c95
Add pointer loading example from comment
kylecarbs Sep 19, 2022
2b87ae1
Fix duplicate test, remove pointer.Handle
kylecarbs Sep 19, 2022
7fd0903
Fix expired license
kylecarbs Sep 19, 2022
c67605b
Add entitlements struct
kylecarbs Sep 19, 2022
b8776cb
Merge branch 'main' into entfactor
kylecarbs Sep 20, 2022
7f9ed39
Fix context passing
kylecarbs Sep 20, 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
13 changes: 0 additions & 13 deletions cli/gitssh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import (
"github.com/stretchr/testify/require"
gossh "golang.org/x/crypto/ssh"

"cdr.dev/slog"

"github.com/coder/coder/cli/clitest"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/codersdk"
Expand Down Expand Up @@ -83,18 +81,7 @@ func prepareTestGitSSH(ctx context.Context, t *testing.T) (*codersdk.Client, str
errC <- cmd.ExecuteContext(ctx)
}()
t.Cleanup(func() { require.NoError(t, <-errC) })

coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
resources, err := client.WorkspaceResourcesByBuild(ctx, workspace.LatestBuild.ID)
require.NoError(t, err)
dialer, err := client.DialWorkspaceAgentTailnet(ctx, slog.Logger{}, resources[0].Agents[0].ID)
require.NoError(t, err)
defer dialer.Close()
require.Eventually(t, func() bool {
_, err = dialer.Ping()
return err == nil
}, testutil.WaitMedium, testutil.IntervalFast)

return agentClient, agentToken, pubkey
}

Expand Down
15 changes: 7 additions & 8 deletions cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,13 @@ func Core() []*cobra.Command {
users(),
versionCmd(),
workspaceAgent(),
features(),
}
}

func AGPL() []*cobra.Command {
all := append(Core(), Server(coderd.New))
all := append(Core(), Server(func(_ context.Context, o *coderd.Options) (*coderd.API, error) {
return coderd.New(o), nil
}))
return all
}

Expand Down Expand Up @@ -548,13 +549,11 @@ func checkWarnings(cmd *cobra.Command, client *codersdk.Client) error {
defer cancel()

entitlements, err := client.Entitlements(ctx)
if err != nil {
return xerrors.Errorf("get entitlements to show warnings: %w", err)
}
for _, w := range entitlements.Warnings {
_, _ = fmt.Fprintln(cmd.ErrOrStderr(), cliui.Styles.Warn.Render(w))
if err == nil {
for _, w := range entitlements.Warnings {
_, _ = fmt.Fprintln(cmd.ErrOrStderr(), cliui.Styles.Warn.Render(w))
}
}

return nil
}

Expand Down
9 changes: 6 additions & 3 deletions cli/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ import (
)

// nolint:gocyclo
func Server(newAPI func(*coderd.Options) *coderd.API) *cobra.Command {
func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, error)) *cobra.Command {
var (
accessURL string
address string
Expand Down Expand Up @@ -489,7 +489,10 @@ func Server(newAPI func(*coderd.Options) *coderd.API) *cobra.Command {
), promAddress, "prometheus")()
}

coderAPI := newAPI(options)
coderAPI, err := newAPI(ctx, options)
if err != nil {
return err
}
defer coderAPI.Close()

client := codersdk.New(localURL)
Expand Down Expand Up @@ -536,7 +539,7 @@ func Server(newAPI func(*coderd.Options) *coderd.API) *cobra.Command {
// These errors are typically noise like "TLS: EOF". Vault does similar:
// https://github.com/hashicorp/vault/blob/e2490059d0711635e529a4efcbaa1b26998d6e1c/command/server.go#L2714
ErrorLog: log.New(io.Discard, "", 0),
Handler: coderAPI.Handler,
Handler: coderAPI.RootHandler,
BaseContext: func(_ net.Listener) context.Context {
return shutdownConnsCtx
},
Expand Down
18 changes: 4 additions & 14 deletions coderd/audit/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@ import (

"cdr.dev/slog"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/features"
"github.com/coder/coder/coderd/httpmw"
"github.com/coder/coder/coderd/tracing"
)

type RequestParams struct {
Features features.Service
Log slog.Logger
Audit Auditor
Log slog.Logger

Request *http.Request
Action database.AuditAction
Expand Down Expand Up @@ -102,15 +101,6 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
params: p,
}

feats := struct {
Audit Auditor
}{}
err := p.Features.Get(&feats)
if err != nil {
p.Log.Error(p.Request.Context(), "unable to get auditor interface", slog.Error(err))
return req, func() {}
}

return req, func() {
ctx := context.Background()
logCtx := p.Request.Context()
Expand All @@ -120,15 +110,15 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request
return
}

diff := Diff(feats.Audit, req.Old, req.New)
diff := Diff(p.Audit, req.Old, req.New)
diffRaw, _ := json.Marshal(diff)

ip, err := parseIP(p.Request.RemoteAddr)
if err != nil {
p.Log.Warn(logCtx, "parse ip", slog.Error(err))
}

err = feats.Audit.Export(ctx, database.AuditLog{
err = p.Audit.Export(ctx, database.AuditLog{
ID: uuid.New(),
Time: database.Now(),
UserID: httpmw.APIKey(p.Request).UserID,
Expand Down
2 changes: 1 addition & 1 deletion coderd/authorize.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type HTTPAuthorizer struct {
// return
// }
func (api *API) Authorize(r *http.Request, action rbac.Action, object rbac.Objecter) bool {
return api.httpAuth.Authorize(r, action, object)
return api.HTTPAuth.Authorize(r, action, object)
}

// Authorize will return false if the user is not authorized to do the action.
Expand Down
45 changes: 22 additions & 23 deletions coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"net/url"
"path/filepath"
"sync"
"sync/atomic"
"time"

"github.com/andybalholm/brotli"
Expand All @@ -24,9 +25,9 @@ import (

"cdr.dev/slog"
"github.com/coder/coder/buildinfo"
"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/awsidentity"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/features"
"github.com/coder/coder/coderd/gitsshkey"
"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/coderd/httpmw"
Expand All @@ -50,6 +51,7 @@ type Options struct {
// CacheDir is used for caching files served by the API.
CacheDir string

Auditor audit.Auditor
AgentConnectionUpdateFrequency time.Duration
AgentInactiveDisconnectTimeout time.Duration
// APIRateLimit is the minutely throughput rate limit per user or ip.
Expand All @@ -68,8 +70,6 @@ type Options struct {
Telemetry telemetry.Reporter
TracerProvider trace.TracerProvider
AutoImportTemplates []AutoImportTemplate
LicenseHandler http.Handler
FeaturesService features.Service

TailnetCoordinator *tailnet.Coordinator
DERPMap *tailcfg.DERPMap
Expand All @@ -80,6 +80,9 @@ type Options struct {

// New constructs a Coder API handler.
func New(options *Options) *API {
if options == nil {
options = &Options{}
}
if options.AgentConnectionUpdateFrequency == 0 {
options.AgentConnectionUpdateFrequency = 3 * time.Second
}
Expand Down Expand Up @@ -117,11 +120,8 @@ func New(options *Options) *API {
if options.TailnetCoordinator == nil {
options.TailnetCoordinator = tailnet.NewCoordinator()
}
if options.LicenseHandler == nil {
options.LicenseHandler = licenses()
}
if options.FeaturesService == nil {
options.FeaturesService = &featuresService{}
if options.Auditor == nil {
options.Auditor = audit.NewNop()
}

siteCacheDir := options.CacheDir
Expand All @@ -142,14 +142,16 @@ func New(options *Options) *API {
r := chi.NewRouter()
api := &API{
Options: options,
Handler: r,
RootHandler: r,
siteHandler: site.Handler(site.FS(), binFS),
httpAuth: &HTTPAuthorizer{
HTTPAuth: &HTTPAuthorizer{
Authorizer: options.Authorizer,
Logger: options.Logger,
},
metricsCache: metricsCache,
Auditor: atomic.Pointer[audit.Auditor]{},
}
api.Auditor.Store(&options.Auditor)
api.workspaceAgentCache = wsconncache.New(api.dialWorkspaceAgentTailnet, 0)
api.derpServer = derp.NewServer(key.NewNode(), tailnet.Logger(options.Logger))
oauthConfigs := &httpmw.OAuth2Configs{
Expand Down Expand Up @@ -218,6 +220,8 @@ func New(options *Options) *API {
})

r.Route("/api/v2", func(r chi.Router) {
api.APIHandler = r

r.NotFound(func(rw http.ResponseWriter, r *http.Request) {
httpapi.Write(rw, http.StatusNotFound, codersdk.Response{
Message: "Route not found.",
Expand Down Expand Up @@ -473,14 +477,6 @@ func New(options *Options) *API {
r.Get("/resources", api.workspaceBuildResources)
r.Get("/state", api.workspaceBuildState)
})
r.Route("/entitlements", func(r chi.Router) {
r.Use(apiKeyMiddleware)
r.Get("/", api.FeaturesService.EntitlementsAPI)
})
r.Route("/licenses", func(r chi.Router) {
r.Use(apiKeyMiddleware)
r.Mount("/", options.LicenseHandler)
})
})

r.NotFound(compressHandler(http.HandlerFunc(api.siteHandler.ServeHTTP)).ServeHTTP)
Expand All @@ -489,17 +485,20 @@ func New(options *Options) *API {

type API struct {
*Options
Auditor atomic.Pointer[audit.Auditor]
HTTPAuth *HTTPAuthorizer

derpServer *derp.Server
// APIHandler serves "/api/v2"
APIHandler chi.Router
// RootHandler serves "/"
RootHandler chi.Router

Handler chi.Router
derpServer *derp.Server
metricsCache *metricscache.Cache
siteHandler http.Handler
websocketWaitMutex sync.Mutex
websocketWaitGroup sync.WaitGroup
workspaceAgentCache *wsconncache.Cache
httpAuth *HTTPAuthorizer

metricsCache *metricscache.Cache
}

// Close waits for all WebSocket connections to drain before returning.
Expand Down
10 changes: 0 additions & 10 deletions coderd/coderd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,6 @@ func TestBuildInfo(t *testing.T) {
require.Equal(t, buildinfo.Version(), buildInfo.Version, "version")
}

// TestAuthorizeAllEndpoints will check `authorize` is called on every endpoint registered.
func TestAuthorizeAllEndpoints(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
a := coderdtest.NewAuthTester(ctx, t, nil)
skipRoutes, assertRoute := coderdtest.AGPLRoutes(a)
a.Test(ctx, assertRoute, skipRoutes)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Without this there are no AGPL tests of authorization on endpoints, which is a loss for AGPL code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I'll fix this up.

func TestDERP(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
Expand Down
Loading