Skip to content
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

Create db and kube web handlers #6672

Merged
merged 4 commits into from
May 5, 2021
Merged

Create db and kube web handlers #6672

merged 4 commits into from
May 5, 2021

Conversation

kimlisa
Copy link
Contributor

@kimlisa kimlisa commented Apr 29, 2021

part of #6466
part of #5086

Description

  • Create database and kubernetes handlers
  • Define database and kubernetes access rules

@kimlisa kimlisa requested a review from awly April 29, 2021 23:26
@kimlisa kimlisa force-pushed the lisa/db-kube-web-handle branch from b8afe5e to fc932db Compare April 29, 2021 23:27
@@ -305,6 +305,12 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*RewritingHandler, error) {
// web context
h.GET("/webapi/sites/:site/context", h.WithClusterAuth(h.getUserContext))

// Database access handlers.
h.GET("/webapi/sites/:site/databases", h.WithClusterAuth(h.siteDatabasesGet))
Copy link
Collaborator

Choose a reason for hiding this comment

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

"site" is obsolete terminology - ok to keep it in handler paths but let's name functions clusterDatabasesGet and clusterKubesGet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed

Desc string `json:"desc"`
// Procotol is the database description.
Procotol string `json:"protocol"`
// Type is the database type, self-hosted or AWS RDS.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Instead of "or AWS RDS" I would just say "or cloud-hosted" which would cover RDS, Redshift, Cloud SQL and anything we'll add in future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

revised

@@ -66,6 +67,10 @@ type userACL struct {
Nodes access `json:"nodes"`
// AppServers defines access to application servers
AppServers access `json:"appServers"`
// DbServers defines access to database servers.
DbServers access `json:"dbServers"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: DBServers for consistency with how it's capitalized in other places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed

@kimlisa kimlisa force-pushed the lisa/db-kube-web-handle branch from f9753d9 to 099fd45 Compare April 30, 2021 16:58

// MakeKubes creates ui kube objects and returns a list..
func MakeKubes(clusterName string, servers []types.Server) []Kube {
uiKubeClusters := []Kube{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
uiKubeClusters := []Kube{}
uiKubeClusters := make([]Kube, 0, len(servers))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i didn't make this change, b/c the len is determine by how many kube clusters there are in each server


// MakeDatabases creates server database objects.
func MakeDatabases(clusterName string, servers []types.DatabaseServer) []Database {
uiServers := []Database{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
uiServers := []Database{}
uiServers := make([]Database, 0, len(servers))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

revised

Copy link
Contributor

@russjones russjones left a comment

Choose a reason for hiding this comment

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

Bot.

kimlisa added 4 commits May 5, 2021 12:04
- rename handler name prefix "site" to "cluster"
- rename DbServers to DBServers
- removed URI field b/c it is not needed in front
- update test
@kimlisa kimlisa force-pushed the lisa/db-kube-web-handle branch from 0deff3c to a3e9b25 Compare May 5, 2021 19:04
@kimlisa kimlisa merged commit 9910598 into master May 5, 2021
@kimlisa kimlisa deleted the lisa/db-kube-web-handle branch May 5, 2021 19:52
kimlisa added a commit that referenced this pull request May 5, 2021
kimlisa added a commit that referenced this pull request May 6, 2021
…#6754)

* Create GET db and kube list web handlers (#6672)
* Check cloud feature before setting billing access for web (#6537)
 - Init web handler with auth server feature flags on proxy init
 - Retrieve auth server features by calling Ping when connecting 
  to auth svc which contains the server feature flags in the response
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants