[infra-proxy-service] Add infra-proxy-service & manage servers & orgs APIs #2173

kvivek1115 · 2019-11-12T15:19:50Z

🔩 Description: What code changed, and why?

Implement a new service that is managing the chef servers ends points and teach how to fetch data of the desired state.

As of now, we are dealing with current state data of chef node which might fails and loss of data if the particular node/server fails during convergence.

In order to make a single source of truth for chef infra servers data here, we have added infra-proxy-service service.

In this PR following points I have covered:

Connect infra-proxy-service to deployment-service.
Provide gPRC and HTTP endpoints to manage chef infra servers & orgs.
CRUD for infra servers.
CRUD for infra server orgs.

Pending tasks:

Add IAM policies for admin users only.
Manage infra server user's KEYS using secrets-service.
Add Unit & integration test cases.
Documentations of infra-proxy-service

As I am a newbie to Automate and I have tried to cover as much as possible in order to get the understating of Automate code flow.

Please reviews and do provide the feedback.

Also, I am very thankful to @stevendanna who guided me with proceedings.

Thanks

⛓️ Related Resources

#1544

👍 Definition of Done

Added infra-proxy-service a separate service in order to manage chef infra servers endpoints.

👟 How to Build and Test the Change

build components/infra-proxy-service
rebuild components/automate-gateway
rebuild components/automate-deployment && start_all_services
run chef-automate dev grpcurl automate-gateway list chef.automate.api.infra_proxy.InfraProxy in order to get list of implemeted gRPC APIs.

✅ Checklist

Tests added/updated?
Docs added/updated?

Attached DB structure for reference:

stevendanna

This looks like a pretty good start on this service.

I've left some initial comments that are mostly just small cleanups.

stevendanna · 2019-11-13T01:24:24Z

components/infra-proxy-service/cmd/infra-proxy-service/commands/serve.go

+
+var serveCmd = &cobra.Command{
+	Use:   "serve",
+	Short: fmt.Sprintf("Launches the automate infra proxy service."),


[minor] No need for Sprintf here

stevendanna · 2019-11-13T01:24:53Z

components/infra-proxy-service/cmd/infra-proxy-service/commands/serve.go

+func serve(cmd *cobra.Command, args []string) {
+	cmd.PersistentFlags().StringP("log-level", "l", "info", "log level")
+	cmd.PersistentFlags().StringP("log-format", "f", "text", "log format")
+	cmd.PersistentFlags().String("grpc", "127.0.0.1:9093", "grpc host and port")


[nit] Default port here is different from service's default

Ohh, yup need to update it to port 10153 thanks!

stevendanna · 2019-11-13T01:26:08Z

components/infra-proxy-service/cmd/infra-proxy-service/commands/serve.go

+	u, err := url.Parse(cfg.PGURL)
+	if err != nil {
+		msg := fmt.Sprintf("could not parse pg_url %s from config", cfg.PGURL)
+		fail(errors.Wrap(err, msg))


[minor] You can use errors.Wrapf rather than errors.Wrap+fmt.Sprintf

stevendanna · 2019-11-13T01:30:11Z

components/infra-proxy-service/config/config.go

@@ -0,0 +1,79 @@
+package config


Is any of this file used elsewhere? Can we remove it? The service builds fine if I delete the file.

Nope, we haven't used it anywhere. after removing it seems all working fine. 👍

components/infra-proxy-service/server/v1/orgs.go

stevendanna · 2019-11-13T01:50:09Z

components/infra-proxy-service/storage/postgres/migration/sql/00_add_infra_proxy.up.sql

+  id           uuid PRIMARY KEY,
+  name         TEXT NOT NULL DEFAULT '',
+  admin_user   TEXT NOT NULL DEFAULT '', /* Added for now but can be thing of managing mutitple keys & users */
+  admin_key    TEXT NOT NULL DEFAULT '',


Should consider storing admin_key in secrets-service?

Yes, this definitely will store in a secrets-service. I have kept it in my TODO list.

stevendanna · 2019-11-13T01:52:56Z

components/infra-proxy-service/storage/postgres/postgres.go

+)
+
+// TODOs
+// - CREATE EXTENSION "uuid-ossp"; (currently done via Makefile for tests)


This is done in your run hook now so it looks like you can remove the TODO

stevendanna · 2019-11-13T01:54:52Z

components/infra-proxy-service/storage/postgres/servers.go

+	var s storage.Server
+	err := p.db.QueryRowContext(ctx,
+		`SELECT s.id, s.name, s.description, s.fqdn, s.ip_address, s.updated_at, s.created_at,
+		COALESCE((SELECT count(*) FROM orgs o WHERE o.server_id = s.id), 0) AS orgs_count


Do we need to COALESCE here? Won't the SELECT count(*) return 0 if no conditions are met?

Nope, we don't need COALESCE here as such just put an extra layer 😄 will remove it thanks

stevendanna · 2019-11-13T01:58:15Z

components/infra-proxy-service/habitat/hooks/init

+# Please put potentially long-running and/or complex operations in the run hook rather
+# than the init hook until the issue[0] has been resolved.
+#
+# [0] https://github.com/habitat-sh/habitat/issues/1973


I think let's just exclude this hook altogether I think.

stevendanna · 2019-11-13T01:58:35Z

components/infra-proxy-service/habitat/default.toml

+[service]
+host = "localhost"
+port = 10153
+external_fqdn = "localhost"


I don't think this service uses external_fqdn for anything.

stevendanna · 2019-11-13T08:47:11Z

api/config/infra_proxy/config_request.proto

+	option (chef.automate.api.service_config) = {name: "infra-proxy-service"};
+
+	reserved 1 to 2;
+	V1 v1 = 3;


[minor] Since this is a brand new proto, no need to start with fields 1 and 2 as reserved, this can be V1 v1 =1 and you can removed reserved 1 to 2; above (this will require rebuilding config protos.

stevendanna · 2019-11-13T13:27:00Z

#2189

jaym · 2019-11-18T12:57:19Z

components/infra-proxy-service/cmd/infra-proxy-service/commands/serve.go

+		}
+	}
+
+	closer, err := tracing.NewGlobalTracer("infra-proxy")


we dont use any of this tracing stuff. We should probably stop putting it in

Okay sure, will check and update it accordingly thanks pointing it out.

jaym · 2019-11-18T13:07:53Z

components/infra-proxy-service/server/v1/orgs.go

+
+// CreateOrg creates a new org
+func (s *Server) CreateOrg(ctx context.Context, req *request.CreateOrg) (*response.CreateOrg, error) {
+	ctx, cancel := context.WithCancel(ctx)


cant remember, what's the reason for doing this

@jaym I think in interservice we have defined interfaces that need to be implemented by the infra proxy server.

I think this is so that when this function is left all the processes created from this function stop.

kvivek1115 · 2020-01-29T05:28:24Z

This PR is good to go. I have added a couple of changes after the last review.

Regarding policy changes, have renamed infra_proxy prefix with infra as discussed.

Note: IAM Projects mapping and migrates existing ingest policies are pending will do in further PR.

lancewf · 2020-01-30T18:26:25Z

components/infra-proxy-service/Makefile

+	../../scripts/generate_and_check_migration_files.sh $@
+
+# Regenerate all *.pb.go files
+proto:


I don't think this is needed because all the protos for the infra-proxy-service are in the api folder.

Somehow it left untouched, have removed it thanks

lancewf · 2020-01-30T18:38:07Z

components/infra-proxy-service/habitat/plan.sh

+  chef/mlsa
+)
+pkg_exports=(
+  [port]=service.port # default service is grpc


I think you might want to add a [host]=service.host to the pkg_exports like below.

pkg_exports=( [port]=service.port [host]=service.host )

This allows other services to know how to connect to this service through localhost or the IP address.

Yes, thanks for pointing it out.

lancewf · 2020-01-30T19:12:17Z

go.mod

@@ -106,7 +107,7 @@ require (
 	github.com/spf13/jwalterweatherman v0.0.0-20170901151539-12bd96e66386 // indirect
 	github.com/spf13/pflag v1.0.1
 	github.com/spf13/viper v1.0.1-0.20171207042631-1a0c4a370c3e
-	github.com/stretchr/testify v1.3.0
+	github.com/stretchr/testify v1.4.0


What is the reason to upgrade this library?

I think while updating the go-chef vendor library, have found minor update in testify lib as well, so updated in this PR only.

Signed-off-by: Steven Danna <[email protected]> (cherry picked from commit 34cd81d)

- Cover MSYS-1155 specifications. - Add gRPC proto request and & response messages - Add config request proto message. - Register InfraProxyServer with server cmd. - Connect gRPC server functions to DB functions in order to manage servers CRUD. - Write a habitat plan to create DB & migrations. - Add health-check and init hab template. - Add hab config.yml Signed-off-by: Vivek Singh <[email protected]>

Signed-off-by: Vivek Singh <[email protected]>

- Rename v1 server.go to servers.go. - Modify request & response message format. Signed-off-by: Vivek Singh <[email protected]>

Signed-off-by: Steven Danna <[email protected]>

- Add infra-proxy-service backup spec - Append server_id in GetOrgs & other orgs API. - Modify query & other methods. Signed-off-by: Vivek Singh <[email protected]>

- Update orgs_count automate-gateway GetSevers API. - Remove unused code. Signed-off-by: Vivek Singh <[email protected]>

Signed-off-by: Vivek Singh <[email protected]>

…nges Signed-off-by: Vivek Singh <[email protected]>

Signed-off-by: Vivek Singh <[email protected]>

- make revendor to update the dependency. Signed-off-by: Vivek Singh <[email protected]>

- Add GetCookbooks & GetCookbooksAvailalbeVersions grpc & http API. Signed-off-by: Vivek Singh <[email protected]>

- Fix minor reviews comments. Signed-off-by: Vivek Singh <[email protected]>

Signed-off-by: Vivek Singh <[email protected]>

- https://github.com/chef/go-chef - minor update regarding server delete the message - constructing the base URL based on fqdn|ipAddress value Signed-off-by: Vivek Singh <[email protected]>

…vers Signed-off-by: Vivek Singh <[email protected]>

- Connect infra-proxy-service with secrets-service. - Access secrets-service API in order to manage the admin key. Signed-off-by: Vivek Singh <[email protected]>

- Update UpdateOrg, DeleteOrg and GetOrg in order manage key. Signed-off-by: Vivek Singh <[email protected]>

Signed-off-by: Vivek Singh <[email protected]>

kvivek1115 self-assigned this Nov 12, 2019

stevendanna reviewed Nov 13, 2019

View reviewed changes

kvivek1115 force-pushed the VSingh/automate-infra-proxy-base branch 2 times, most recently from 5cb31e7 to c7732a9 Compare November 14, 2019 05:34

jaym reviewed Nov 18, 2019

View reviewed changes

kvivek1115 force-pushed the VSingh/automate-infra-proxy-base branch 2 times, most recently from 38f0739 to fc4f34d Compare December 1, 2019 06:34

kvivek1115 force-pushed the VSingh/automate-infra-proxy-base branch 3 times, most recently from 529f839 to c36433b Compare December 9, 2019 10:39

kvivek1115 force-pushed the VSingh/automate-infra-proxy-base branch 3 times, most recently from 6dfe59d to 3e76364 Compare December 24, 2019 10:18

sachin-msys mentioned this pull request Dec 27, 2019

Sachin/automate infra proxy server ui #2581

Merged

kvivek1115 force-pushed the VSingh/automate-infra-proxy-base branch from 3e76364 to b2feecd Compare January 3, 2020 07:13

kvivek1115 requested review from tylercloke and lancewf January 7, 2020 03:40

kvivek1115 mentioned this pull request Jan 16, 2020

[deployment-service] make update-bindings notable changes #2600

Merged

2 tasks

susanev added the automate-infra-proxy label Jan 24, 2020

kvivek1115 changed the title ~~[WIP] [infra-proxy-service] Add infra-proxy-service & manage servers & orgs APIs~~ [infra-proxy-service] Add infra-proxy-service & manage servers & orgs APIs Jan 28, 2020

kvivek1115 changed the title ~~[infra-proxy-service] Add infra-proxy-service & manage servers & orgs APIs~~ [WIP] [infra-proxy-service] Add infra-proxy-service & manage servers & orgs APIs Jan 28, 2020

kvivek1115 force-pushed the VSingh/automate-infra-proxy-base branch from 95c4a2f to dcc91da Compare January 29, 2020 04:15

kvivek1115 changed the title ~~[WIP] [infra-proxy-service] Add infra-proxy-service & manage servers & orgs APIs~~ [infra-proxy-service] Add infra-proxy-service & manage servers & orgs APIs Jan 29, 2020

lancewf reviewed Jan 30, 2020

View reviewed changes

stevendanna and others added 27 commits February 17, 2020 14:40

initialize connfactory

2d5df5f

Signed-off-by: Steven Danna <[email protected]> (cherry picked from commit 34cd81d)

Rename service infra-proxy to infra-proxy-service

e5045d1

Signed-off-by: Vivek Singh <[email protected]>

Add infra proxy to automate-gateway server

41e7314

Signed-off-by: Vivek Singh <[email protected]>

Add orgs_count & description for the infra server

d43928d

- Rename v1 server.go to servers.go. - Modify request & response message format. Signed-off-by: Vivek Singh <[email protected]>

infra-proxy-service: wire into deployment-service

a0fe8ba

Signed-off-by: Steven Danna <[email protected]>

Move infra proxy API to external

b5671a7

- Add infra-proxy-service backup spec - Append server_id in GetOrgs & other orgs API. - Modify query & other methods. Signed-off-by: Vivek Singh <[email protected]>

Incorporated reviews comment

dc17ee1

- Update orgs_count automate-gateway GetSevers API. - Remove unused code. Signed-off-by: Vivek Singh <[email protected]>

Minor updates

d232077

Signed-off-by: Vivek Singh <[email protected]>

Protobuff changes

fa38a75

Signed-off-by: Vivek Singh <[email protected]>

bldr config gen

bb332d6

Signed-off-by: Vivek Singh <[email protected]>

infra-proxy-service: Fix package meta

f2fecb8

Signed-off-by: Vivek Singh <[email protected]>

infra-proxy-service: Generate gateway clients mockgen

e383652

Signed-off-by: Vivek Singh <[email protected]>

infra-proxy-service: re-run make generate to reflect package meta cha…

e68f739

…nges Signed-off-by: Vivek Singh <[email protected]>

infra-proxy-service: add uses_platform_scaffolding: true

9558851

Signed-off-by: Vivek Singh <[email protected]>

Add go-chef Chef server API go client

5ef2321

- make revendor to update the dependency. Signed-off-by: Vivek Singh <[email protected]>

Add proxy module in order to fetch chef-server data fron go-client

641264b

- Add GetCookbooks & GetCookbooksAvailalbeVersions grpc & http API. Signed-off-by: Vivek Singh <[email protected]>

Update ParseStorageError for the storage error.

6547381

- Fix minor reviews comments. Signed-off-by: Vivek Singh <[email protected]>

infra-proxy-service: Add GetCookbook API

c87fedf

Signed-off-by: Vivek Singh <[email protected]>

[infra-proxy-service] Update go-chef module with chef forked repo

19ef14a

- https://github.com/chef/go-chef - minor update regarding server delete the message - constructing the base URL based on fqdn|ipAddress value Signed-off-by: Vivek Singh <[email protected]>

[infra-proxy-service] rename route /infra_proxy/servers to /infra/ser…

398f451

…vers Signed-off-by: Vivek Singh <[email protected]>

Manage org key using secrets-service

3bfcfd6

- Connect infra-proxy-service with secrets-service. - Access secrets-service API in order to manage the admin key. Signed-off-by: Vivek Singh <[email protected]>

Manage org admin key using secrets service update

f4c813e

- Update UpdateOrg, DeleteOrg and GetOrg in order manage key. Signed-off-by: Vivek Singh <[email protected]>

[infra-proxy-service] minor review comments

329bf09

Signed-off-by: Vivek Singh <[email protected]>

[infra-proxy-service] make update-bindings

df9718a

Signed-off-by: Vivek Singh <[email protected]>

[infra-proxy-service] remove TODO comment

bf87308

Signed-off-by: Vivek Singh <[email protected]>

[infra-proxy-service] add service call before reflection

9c11c0a

Signed-off-by: Vivek Singh <[email protected]>

kvivek1115 force-pushed the VSingh/automate-infra-proxy-base branch from 34ed861 to 9c11c0a Compare February 17, 2020 12:33

[infra-proxy-service] fix make lint

53b50aa

Signed-off-by: Vivek Singh <[email protected]>

susanev merged commit e7b05c1 into chef:master Feb 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[infra-proxy-service] Add infra-proxy-service & manage servers & orgs APIs #2173

[infra-proxy-service] Add infra-proxy-service & manage servers & orgs APIs #2173

kvivek1115 commented Nov 12, 2019

stevendanna left a comment

stevendanna Nov 13, 2019

stevendanna Nov 13, 2019

kvivek1115 Nov 13, 2019

stevendanna Nov 13, 2019

stevendanna Nov 13, 2019

kvivek1115 Nov 13, 2019

stevendanna Nov 13, 2019

kvivek1115 Nov 13, 2019

stevendanna Nov 13, 2019

stevendanna Nov 13, 2019

kvivek1115 Nov 13, 2019 •

edited

Loading

stevendanna Nov 13, 2019

stevendanna Nov 13, 2019

stevendanna Nov 13, 2019

stevendanna commented Nov 13, 2019

jaym Nov 18, 2019

kvivek1115 Nov 18, 2019

jaym Nov 18, 2019

kvivek1115 Nov 25, 2019

lancewf Jan 30, 2020

kvivek1115 commented Jan 29, 2020

lancewf Jan 30, 2020

kvivek1115 Jan 31, 2020

lancewf Jan 30, 2020

kvivek1115 Jan 31, 2020

lancewf Jan 30, 2020

kvivek1115 Jan 31, 2020

[infra-proxy-service] Add infra-proxy-service & manage servers & orgs APIs #2173

[infra-proxy-service] Add infra-proxy-service & manage servers & orgs APIs #2173

Conversation

kvivek1115 commented Nov 12, 2019

🔩 Description: What code changed, and why?

In this PR following points I have covered:

Pending tasks:

⛓️ Related Resources

👍 Definition of Done

👟 How to Build and Test the Change

✅ Checklist

stevendanna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kvivek1115 Nov 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevendanna commented Nov 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kvivek1115 commented Jan 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kvivek1115 Nov 13, 2019 •

edited

Loading