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

In-product site configuration #965

Closed
sqs opened this Issue Nov 13, 2018 · 13 comments

Comments

Projects
None yet
3 participants
@sqs
Copy link
Member

sqs commented Nov 13, 2018

Site configuration is how site admins configure things for their Sourcegraph instance:

  • repository mirroring (from GitHub, GitLab, and other code hosts)
  • user authentication providers
  • email (SMTP credentials)
  • and other things

The full list of things is in site.schema.json.

There are 3 problems with this that we want to solve, in order of importance:

  • When running a Sourcegraph cluster, you can't edit the site config on the web. You need to edit a k8s configmap and then wait for a reload (or manually kill pods).
  • Sourcegraph can't automatically migrate the site config content when fields change. (For example, the auth.saml -> auth.provider -> auth.providers change was painful.) This is true in both sourcegraph/server and cluster, because the config file is "owned" by the site admin, not by Sourcegraph.
  • Tech debt: Propagating frontend values to other services (such as the list of gitservers) is weird.
  • (Out of scope right now) Site config contains secrets, such as TLS keys, GitHub tokens, etc., that are sourced from central config management systems and should not be present in unencrypted form on disk (for some customers).

The solution must abide by the following:

  • Currently a site admin on sourcegraph/server can make a change (on sourcegraph/server, and on cluster when we solve the 1st problem) that takes down their site, such as changing appURL. Right now, they can fix the problem by manually editing the site config file, but when we solve the 2nd problem, this might no longer be possible. This needs to not become a problem.
  • It is OK to make certain fields in site configuration harder to edit (such as appURL and auth.providers) in exchange for making most others easier to edit.

Proposed solution:

  1. Move site config that can't take down the site (i.e., all except appURL, auth.providers, etc.) out of the site config file and into "core site config", and make both be stored in the DB.
  2. Make a new management console UI that allows editing the core config (the site admin page in-app will only allow editing the site config, the management console UI must be used to edit core config).
  3. The management console UI has extra work needed around authentication, TBD at a later date once initial management console UI is complete.
  4. (later?) Get #929 finished to remove more things from the "dangerous site config".

Decisions:

  • "Deployment config" (PG*, REDIS_*, SRC_GITSERVERS) is renamed to "service connections".

Levels:

  • User settings
  • Org settings
  • Global settings
  • Site configuration
  • Core configuration -> Critical configuration
  • Deployment configuration -> Cluster service connections

Discussion:

  • Is it important to store the "dangerous" site config outside the database? That lets you store "deployment configuration" in the config file as well.
    • Deployment configuration: PG*, REDIS_*, which gitserver instances to talk to

How other products solve this:

  • GitLab's gitlab.rb config file
    • GitLab deciding on configurations: "When we have no choice, the secondary priority is to configure something in the GitLab interface. A configuration should only appear in a file (gitlab.rb or gitlab.yml) as a last resort."
  • GitHub Enterprise's ini file

Draft announcement (from before kickoff)

Project status:

  • Last week I moved significant portions of #966 into smaller PRs that I can land tomorrow with a strong guarantee of no regressions. (see PRs linked to from that one)
  • I still need to address some TODOs in #966 before it itself can be merged, I am hoping to have that completed by Tue/Wed.
  • The management-console itself still needs a UX (even if primitive / just a textarea for now). I am hoping to have the primitive version done late Wed / mid Thur.

@sqs sqs added this to the 3.0-preview milestone Nov 13, 2018

@sqs

This comment has been minimized.

Copy link
Member Author

sqs commented Nov 13, 2018

FYI @slimsag I added this constraint on the solution:

If someone is just running Sourcegraph on localhost using sourcegraph/server and does not need to change any critical config values from the defaults (which are the same as today, specifically defaulting to the builtin username-password auth provider), they should not need to access the management console UI at all. (This lets us retain the nice flow to try Sourcegraph of "run a single docker run command and go to http://localhost:7080").

and rewrote these 2 bullets in the proposed solution:

  • In the management console UI, add a Monaco editor with a JSON Schema for the critical config.
  • Figure out the authentication story for the management console UI. Use a simple password, make its port only accessible by using kubectl port-forward and similar for sourcegraph/server, etc.?

@sqs sqs changed the title WIP: In-product site configuration In-product site configuration Nov 13, 2018

@slimsag

This comment has been minimized.

Copy link
Member

slimsag commented Nov 19, 2018

Update:

  • Most significant work is complete in #966 excluding the actual web UI for the management console.
  • This week I will be trying to merge as much of that change as I can, by factoring some parts out into smaller-in-scope PRs (the PR has gotten too large to faithfully review/merge).
  • Management console web UI expected to be out Wed/Thurs.

slimsag added a commit that referenced this issue Nov 21, 2018

mv dbconn to pkg/dbconn
With the introduction of the management console, the frontend will no longer be
the only service which talks to the database. Therefor, sharing the logic to
connect to and migrate the database makes sense. This change does just that.

It was also sharable before, but it would have meant importing
`cmd/frontend/db/dbconn` from the management console which does not make sense
logically as they are separate services.

Migration logic still lives in pkg/dbconn, because we only have one database
(not multiple).

Helps #965

slimsag added a commit that referenced this issue Nov 21, 2018

mv dbconn to pkg/dbconn
With the introduction of the management console, the frontend will no longer be
the only service which talks to the database. Therefor, sharing the logic to
connect to and migrate the database makes sense. This change does just that.

It was also sharable before, but it would have meant importing
`cmd/frontend/db/dbconn` from the management console which does not make sense
logically as they are separate services.

Migration logic still lives in pkg/dbconn, because we only have one database
(not multiple).

Helps #965

slimsag added a commit that referenced this issue Nov 21, 2018

mv dbconn to pkg/dbconn
With the introduction of the management console, the frontend will no longer be
the only service which talks to the database. Therefor, sharing the logic to
connect to and migrate the database makes sense. This change does just that.

It was also sharable before, but it would have meant importing
`cmd/frontend/db/dbconn` from the management console which does not make sense
logically as they are separate services.

Migration logic still lives in pkg/dbconn, because we only have one database
(not multiple).

Helps #965

slimsag added a commit that referenced this issue Nov 21, 2018

Squashed commit of the following:
commit 32a3cf3
Merge: 7852328 a6c4ce8
Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
Date:   Tue Nov 20 20:35:39 2018 -0700

    Merge branch 'sg/merged' into management-console

commit a6c4ce8
Merge: 1567da2 f7403d3
Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
Date:   Tue Nov 20 20:35:34 2018 -0700

    Merge branch 'sg/pkg-dbconn' into sg/merged

commit f7403d3
Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
Date:   Tue Nov 20 20:33:45 2018 -0700

    mv cmd/frontend/db/migrations -> migrations/

    Previously the generated contents of the migrations/ folder was stored
    in a subpackage of the frontend. This no longer makes sense with pkg/dbconn
    importing it.

    Additionally, this is something that confuses me every time I add a migration; i.e.
    "what directory do I generate and why is it not the same as the directory I modified?!"

commit 7852328
Merge: e33160a a0cb93d
Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
Date:   Tue Nov 20 20:29:36 2018 -0700

    Merge branch 'sg/merged' into management-console

    Conflicts:
    	enterprise/cmd/frontend/db/pkgs.go
    	enterprise/cmd/frontend/db/pkgs_test.go

commit a0cb93d
Merge: 1567da2 0e74e0e
Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
Date:   Tue Nov 20 20:28:45 2018 -0700

    Merge branch 'sg/pkg-dbconn' into sg/merged

commit 0e74e0e
Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
Date:   Tue Nov 20 20:11:47 2018 -0700

    mv dbconn to pkg/dbconn

    With the introduction of the management console, the frontend will no longer be
    the only service which talks to the database. Therefor, sharing the logic to
    connect to and migrate the database makes sense. This change does just that.

    It was also sharable before, but it would have meant importing
    `cmd/frontend/db/dbconn` from the management console which does not make sense
    logically as they are separate services.

    Migration logic still lives in pkg/dbconn, because we only have one database
    (not multiple).

    Helps #965

commit e33160a
Merge: cccd431 8a22a43
Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
Date:   Tue Nov 20 19:43:32 2018 -0700

    Merge remote-tracking branch 'origin/master' into management-console

    Conflicts:
    	cmd/frontend/db/global_state.go
    	cmd/frontend/db/global_state_mock.go
    	cmd/frontend/db/global_state_test.go
    	cmd/frontend/db/migrations/bindata.go
    	cmd/frontend/db/mockstores.go
    	cmd/frontend/db/schema.md
    	cmd/frontend/db/site_config.go
    	cmd/frontend/db/site_config_mock.go
    	cmd/frontend/db/site_config_test.go
    	cmd/frontend/db/site_id_info.go
    	cmd/frontend/db/site_id_mock.go
    	cmd/frontend/db/site_info_test.go
    	cmd/frontend/db/stores.go
    	cmd/frontend/db/users.go
    	cmd/frontend/db/users_test.go
    	cmd/frontend/internal/app/jscontext/jscontext.go
    	cmd/frontend/internal/pkg/siteid/siteid.go
    	cmd/frontend/internal/pkg/siteid/siteid_test.go
    	cmd/frontend/types/types.go
    	migrations/1528395560_.down.sql
    	migrations/1528395560_.up.sql

commit cccd431
Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
Date:   Tue Nov 20 13:58:36 2018 -0700

    Squashed commit of the following:

    commit 353be8b
    Merge: c1b1e12 450286d
    Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
    Date:   Tue Nov 20 13:35:51 2018 -0700

        Merge remote-tracking branch 'origin/master' into management-console

        Conflicts:
        	cmd/frontend/shared/authz_test.go
        	enterprise/cmd/frontend/db/pkgs.go
        	enterprise/cmd/frontend/db/pkgs_test.go

    commit c1b1e12
    Merge: 269f630 0239c4b
    Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
    Date:   Fri Nov 16 15:47:14 2018 -0700

        Merge branch 'master' into management-console

        Conflicts:
        	cmd/frontend/db/migrations/bindata.go
        	enterprise/cmd/frontend/internal/registry/extensions_db.go
        	enterprise/cmd/frontend/internal/registry/releases_db.go
        	migrations/1528395559_.down.sql
        	migrations/1528395559_.up.sql
        	schema/site.schema.json
        	schema/site_stringdata.go

    commit 269f630
    Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
    Date:   Fri Nov 16 15:38:45 2018 -0700

        wip

    commit 01b4b49
    Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
    Date:   Thu Nov 15 02:16:02 2018 -0700

        defaults

    commit b10e1dd
    Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
    Date:   Thu Nov 15 02:15:48 2018 -0700

        passthrough

    commit 09735f8
    Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
    Date:   Thu Nov 15 01:57:52 2018 -0700

        confdb: add default site configuration support

    commit 4c691b6
    Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
    Date:   Wed Nov 14 13:37:02 2018 -0700

        remove SiteConfiguration ID logic for now (I do not need to solve this problem today)

    commit 173700e
    Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
    Date:   Tue Nov 13 21:42:07 2018 -0700

        wip

    commit 95f9961
    Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
    Date:   Tue Nov 13 21:41:09 2018 -0700

        Squashed commit of the following:

        commit 4195af1
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Tue Nov 13 21:36:58 2018 -0700

            fix test configuration seeding

        commit ed44198
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Tue Nov 13 02:13:51 2018 -0700

            fix more merge issues

        commit 710c300
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Tue Nov 13 01:49:26 2018 -0700

            fix another conf.Get().Core reference

        commit ce11e16
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Tue Nov 13 01:48:22 2018 -0700

            fix RawUnifiedConfiguration construction

        commit 928f26c
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Tue Nov 13 01:47:51 2018 -0700

            fix bad merge / httpapi routing

        commit 9b605a5
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Tue Nov 13 01:47:33 2018 -0700

            fix validation

        commit e7a7889
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Tue Nov 13 01:20:08 2018 -0700

            update conf.Get().Core callsites

        commit 36d9be8
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Tue Nov 13 01:18:18 2018 -0700

            fetch config from frontend

        commit 395e5da
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Fri Nov 9 17:15:35 2018 -0700

            remove env var support

        commit 5c14d09
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Fri Nov 9 17:03:45 2018 -0700

            - Remove `fetcher` abstraction interface (no longer required).
            - Fetch raw unified config via api.InternalClient

        commit fa9d4d8
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Tue Nov 13 00:19:33 2018 -0700

            gofmt

        commit 803f92f
        Merge: fe2227d bbe26f2
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Mon Nov 12 23:37:52 2018 -0700

            Merge remote-tracking branch 'origin/master' into management-console

            Conflicts:
            	cmd/frontend/db/site_id_info.go
            	cmd/frontend/internal/cli/http.go
            	cmd/frontend/internal/cli/middleware/redirect.go
            	cmd/frontend/internal/cli/middleware/redirect_test.go
            	cmd/frontend/internal/cli/serve_cmd.go
            	cmd/frontend/internal/httpapi/router/router.go
            	cmd/frontend/internal/pkg/siteid/siteid_test.go
            	cmd/frontend/registry/extensions.go
            	cmd/frontend/shared/authz.go
            	enterprise/cmd/frontend/auth/openidconnect/config.go
            	enterprise/cmd/frontend/auth/openidconnect/config_test.go
            	enterprise/cmd/frontend/auth/saml/config.go
            	enterprise/cmd/frontend/auth/saml/config_test.go
            	enterprise/cmd/frontend/auth/saml/middleware_test.go
            	enterprise/cmd/frontend/auth/saml/provider.go
            	enterprise/cmd/frontend/internal/registry/extension_bundle.go
            	enterprise/cmd/frontend/internal/registry/extension_connection_graphql_test.go
            	enterprise/cmd/frontend/internal/registry/extension_manifest.go
            	enterprise/cmd/frontend/internal/registry/http_api.go
            	pkg/conf/parse.go
            	pkg/conf/parse/diff_test.go
            	schema/schema.go
            	schema/site.schema.json
            	schema/site_stringdata.go

        commit fe2227d
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Fri Nov 9 14:16:17 2018 -0700

            gofmt

        commit ef5e87c
        Author: Geoffrey Gilmore <tkdmaster123@gmail.com>
        Date:   Fri Nov 9 13:14:34 2018 -0800

            wip: create internal api routes & update graphql schema for core and site configuration files (#881)

            * httpapi & pkg/api: create routes for core and site configuration files

            * update graphql schema definitions for site configuration

            * also make change to frontend

            * add graphqlbackend change / TODO

        commit 1b2e2d8
        Author: Stephen Gutekanst <stephen.gutekanst@gmail.com>
        Date:   Fri Nov 9 13:20:06 2018 -0700

            introduce conf.UnifiedConfiguration and nearly fix management-console branch (#906)

            * all: use conf.Get().Core in various locations

            * pkg/conf: introduce UnifiedConfiguration (frontend now builds)

            * more UnifiedConfiguration changes + implement diffing

        commit 9d9ab96
        Author: ggilmore <geoffrey@sourcegraph.com>
        Date:   Fri Nov 9 10:28:28 2018 -0800

            remove appURL, auth.public fields that were mistakenly left in site.schema.json

        commit 35b6e1c
        Merge: d7fecbb 29feb2d
        Author: ggilmore <geoffrey@sourcegraph.com>
        Date:   Fri Nov 9 10:22:48 2018 -0800

            Merge remote-tracking branch 'origin/master' into management-console

            Conflicts:
            	cmd/frontend/db/migrations/bindata.go
            	cmd/frontend/db/repos.go
            	migrations/1528395558_.down.sql
            	migrations/1528395558_.up.sql
            	schema/site.schema.json
            	schema/site_stringdata.go

        commit d7fecbb
        Author: Geoffrey Gilmore <tkdmaster123@gmail.com>
        Date:   Thu Nov 8 14:29:41 2018 -0800

            implement basic management console binary (#885)

            * try moving frontend's dbconn to shared package

            * wip - add basic management console implementation

            * add docker directive to top of management console

            * add documentation for management console at top of file

            * add procfile entry

            * add management-console to goreman

            * add management-console to src-prof-services.json

            * add management console to cmd/server/build.sh

            * add management console to cmd/server/shared/globals.go

            * go fmt

            * add management console to enteprise procfile

            * fix spelling error

            * add management console to delve-hooks

            * confdb: have SiteCoreConfigurationFiles use pkg/dbconn.Global instead of a struct field

            * dev/ci: add management-console to all docker images

            * add management-console to cmd/server/shared.go

            * add management-console to go-install.sh

            * add postgres go-dockerize directives to management-console

            * use log15.New to avoid some boilerplate

        commit a49a7de
        Author: ggilmore <geoffrey@sourcegraph.com>
        Date:   Thu Nov 8 08:59:49 2018 -0800

            regenerate schema

        commit 2336135
        Author: Geoffrey Gilmore <tkdmaster123@gmail.com>
        Date:   Wed Nov 7 19:48:14 2018 -0800

            fix typo in migration

        commit 0eea09b
        Author: Geoffrey Gilmore <tkdmaster123@gmail.com>
        Date:   Wed Nov 7 18:56:01 2018 -0800

            implement r/w operations w/ postgres for site and core schemas (#873)

        commit d826c0d
        Author: Geoffrey Gilmore <tkdmaster123@gmail.com>
        Date:   Tue Nov 6 22:44:48 2018 -0800

            schema: move "dangerous" site configuration options to separate "core.schema.json" (#863)

slimsag added a commit that referenced this issue Nov 21, 2018

mv dbconn to pkg/dbconn
With the introduction of the management console, the frontend will no longer be
the only service which talks to the database. Therefor, sharing the logic to
connect to and migrate the database makes sense. This change does just that.

It was also sharable before, but it would have meant importing
`cmd/frontend/db/dbconn` from the management console which does not make sense
logically as they are separate services.

Migration logic still lives in pkg/dbconn, because we only have one database
(not multiple).

Helps #965
@slimsag

This comment has been minimized.

Copy link
Member

slimsag commented Nov 26, 2018

@sqs do you have thoughts around this part:

Figure out the authentication story for the management console UI. Use a simple password, make its port only accessible by using kubectl port-forward and similar for sourcegraph/server, etc.?

@slimsag

This comment has been minimized.

Copy link
Member

slimsag commented Nov 26, 2018

Update:

  • Last week I moved significant portions of #966 into smaller PRs that I can land tomorrow with a strong guarantee of no regressions. (see PRs linked to from that one)
  • I still need to address some TODOs in #966 before it itself can be merged, I am hoping to have that completed by Tue/Wed.
  • The management-console itself still needs a UX (even if primitive / just a textarea for now). I am hoping to have the primitive version done late Wed / mid Thur.
@sqs

This comment has been minimized.

Copy link
Member Author

sqs commented Nov 26, 2018

@slimsag The simple password seems to me to be the safest and simplest, since it can be self contained and is not possible to misconfigure and accidentally expose. (Unless you create a very weak password.) Also, that’s what GHE does, so it works somewhat well at least. So go with that (if you have any better ideas, feel free to bring them up).

@slimsag

This comment has been minimized.

Copy link
Member

slimsag commented Nov 26, 2018

Got it. For new instances we can require admins create it when they create the admin account, but for users migrating from an old version, what kind of experience do we need? Is running some manual SQL query to set the password OK?

@sqs

This comment has been minimized.

Copy link
Member Author

sqs commented Nov 26, 2018

For 3.0-preview, it is OK for the migration path to be rough. We will add a smooth migration (for many things, not just this) in 3.0.

Here is a sketch of the flow here that I think will work. My intent is to make it work for all deployment types, for both instances newly spun up at 3.0-preview and for older instances, and both at the time of initial setup and later. I also want the initial setup flow to NOT need to first go to the mgmt console, which would add another step and probably decrease conversion.

  • Each time the frontend server starts up, it checks for the presence of the mgmt console password. If none exists (which will occur for new instances and for instances upgraded from pre-3.0-preview), it generates a new strong password. It stores the bcrypted form of this password AND the plaintext password itself in the DB.
  • All site admins who view Sourcegraph are prompted (on the site admin overview page) with a box like "Management console password: asdf1234. Copy to clipboard and don't show again" and with a link to the management console
  • Once any site admin clicks Don't show again, the plaintext password is cleared from the DB (the bcrypted form remains, of course).
  • Document a way for site admins to manually reset the password at a psql prompt. The value that they set doesn't need to come from a PostgreSQL query (eg you might want to bcrypt the value in Python or something on the CLI and have them paste that value into an SQL query, instead of requiring/assuming installation of bcrypt for PostgreSQL).

The only other things that might be necessary for 3.0 (not 3.0-preview) are:

  • A way to change the mgmt console password from the mgmt console itself
  • A way to set the mgmt console password in an env var or via k8s secrets instead of in the DB

slimsag added a commit that referenced this issue Nov 27, 2018

mv dbconn to pkg/dbconn
With the introduction of the management console, the frontend will no longer be
the only service which talks to the database. Therefor, sharing the logic to
connect to and migrate the database makes sense. This change does just that.

It was also sharable before, but it would have meant importing
`cmd/frontend/db/dbconn` from the management console which does not make sense
logically as they are separate services.

Migration logic still lives in pkg/dbconn, because we only have one database
(not multiple).

Helps #965

slimsag added a commit that referenced this issue Nov 27, 2018

mv dbconn to pkg/dbconn
With the introduction of the management console, the frontend will no longer be
the only service which talks to the database. Therefor, sharing the logic to
connect to and migrate the database makes sense. This change does just that.

It was also sharable before, but it would have meant importing
`cmd/frontend/db/dbconn` from the management console which does not make sense
logically as they are separate services.

Migration logic still lives in pkg/dbconn, because we only have one database
(not multiple).

Helps #965

slimsag added a commit that referenced this issue Nov 27, 2018

mv cmd/frontend/db/dbconn to pkg/dbconn (#1084)
* mv dbconn to pkg/dbconn

With the introduction of the management console, the frontend will no longer be
the only service which talks to the database. Therefor, sharing the logic to
connect to and migrate the database makes sense. This change does just that.

It was also sharable before, but it would have meant importing
`cmd/frontend/db/dbconn` from the management console which does not make sense
logically as they are separate services.

Migration logic still lives in pkg/dbconn, because we only have one database
(not multiple).

Helps #965

* mv cmd/frontend/db/migrations -> migrations/

Previously the generated contents of the migrations/ folder was stored
in a subpackage of the frontend. This no longer makes sense with pkg/dbconn
importing it.

Additionally, this is something that confuses me every time I add a migration; i.e.
"what directory do I generate and why is it not the same as the directory I modified?!"

* dev: use go generate -x flag for improved CI debugging

* pkg/dbconn: add disclaimer about most services using api.InternalClient instead

* dev/check: add check to ensure pkg/dbconn is only imported by allowed services

* update some imports + editor autofmt query (removes mixed spaces / tabs, uses just tabs now)

* NOCHANGELOG

ryan-blunden added a commit that referenced this issue Nov 28, 2018

mv cmd/frontend/db/dbconn to pkg/dbconn (#1084)
* mv dbconn to pkg/dbconn

With the introduction of the management console, the frontend will no longer be
the only service which talks to the database. Therefor, sharing the logic to
connect to and migrate the database makes sense. This change does just that.

It was also sharable before, but it would have meant importing
`cmd/frontend/db/dbconn` from the management console which does not make sense
logically as they are separate services.

Migration logic still lives in pkg/dbconn, because we only have one database
(not multiple).

Helps #965

* mv cmd/frontend/db/migrations -> migrations/

Previously the generated contents of the migrations/ folder was stored
in a subpackage of the frontend. This no longer makes sense with pkg/dbconn
importing it.

Additionally, this is something that confuses me every time I add a migration; i.e.
"what directory do I generate and why is it not the same as the directory I modified?!"

* dev: use go generate -x flag for improved CI debugging

* pkg/dbconn: add disclaimer about most services using api.InternalClient instead

* dev/check: add check to ensure pkg/dbconn is only imported by allowed services

* update some imports + editor autofmt query (removes mixed spaces / tabs, uses just tabs now)

* NOCHANGELOG

slimsag added a commit that referenced this issue Dec 10, 2018

graphqlbackend: add management console password fetch/clear functiona…
…lity

This will support informing site admins of the management console password and
giving them the ability to copy it. For a logical outline of how this will work
see #965 (comment)

The actual UX which uses these GraphQL queries and mutations will be sent as
part of #966 since it cannot be
merged independently.

Helps #965

slimsag added a commit that referenced this issue Dec 10, 2018

web: components: make CopyableText also support passwords
I am using this component to allow users to copy the management console
password via a UX that looks like this:

![image](https://user-images.githubusercontent.com/3173176/49711146-8d88a480-fbf2-11e8-99c0-7345ea81a4e2.png)

Helps #965

slimsag added a commit that referenced this issue Dec 10, 2018

graphqlbackend: add management console password fetch/clear functiona…
…lity (#1315)

This will support informing site admins of the management console password and
giving them the ability to copy it. For a logical outline of how this will work
see #965 (comment)

The actual UX which uses these GraphQL queries and mutations will be sent as
part of #966 since it cannot be
merged independently.

Helps #965

slimsag added a commit that referenced this issue Dec 10, 2018

web: components: make CopyableText also support passwords (#1316)
I am using this component to allow users to copy the management console
password via a UX that looks like this:

![image](https://user-images.githubusercontent.com/3173176/49711146-8d88a480-fbf2-11e8-99c0-7345ea81a4e2.png)

Helps #965

@nicksnyder nicksnyder modified the milestones: 3.0-preview, 3.0 Dec 14, 2018

@slimsag

This comment has been minimized.

Copy link
Member

slimsag commented Dec 14, 2018

  1. A way to change the mgmt console password from the mgmt console itself
  2. A way to set the mgmt console password in an env var or via k8s secrets instead of in the DB

@sqs If you can make a strong argument for your second point here, I am happy to hear it. I've considered both just now, however, and what I think:

  1. You really should only use the mgmt console passwords we generate securely for you. i.e., passwords that are not >= 128 chars in length will be insecure in nature due to no auth rate limiting. This means we should not do #2 above.
  2. The mgmt console UI will let you regenerate the password, but it'll be automatic / you don't get to pick one. #1438
@sqs

This comment has been minimized.

Copy link
Member Author

sqs commented Dec 14, 2018

@slimsag Sounds good to me. 👍

@slimsag

This comment has been minimized.

Copy link
Member

slimsag commented Dec 14, 2018

Filed #1438 for the UI to regenerate the password, but backlogged it given the pgsql command will do fine in the meantime.

@slimsag

This comment has been minimized.

Copy link
Member

slimsag commented Dec 14, 2018

UPDATE: The management console is merged and working on Sourcegraph.com and dogfood as expected. Migration was automagic, etc.

Issues with config in general which MUST be solved for 3.0:

Issues with config in general which are ideally solved for 3.0, but can slip to later:

I am closing this issue in favor of the above, given the vast bulk of the work here is finished :)

@slimsag slimsag closed this Dec 14, 2018

@slimsag slimsag referenced this issue Dec 14, 2018

Merged

Configuration refactoring + management console #966

33 of 45 tasks complete
@nicksnyder

This comment has been minimized.

Copy link
Member

nicksnyder commented Dec 14, 2018

@slimsag instead of using "idealistic-milestone" can you use "release-blocker" for the ones that are required?

@slimsag

This comment has been minimized.

Copy link
Member

slimsag commented Dec 14, 2018

Yes, I tagged the required ones as release-blocker, but I've left the idealistic milestone because I do want to communicate that it is ideal but perhaps not possible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment