Use new NGINX Plus API #7

Dean-Coakley · 2018-06-15T10:13:03Z

Nginx Plus sdk needs to be updated before /upstream_conf and /status apis are deprecated in R16 in favour of a unified /api endpoint

pleshakov

Requested a couple of changes. Thanks

pleshakov · 2018-06-22T17:15:26Z

README.md

-  * The on-the-fly API is available at **127.0.0.1:8080/upstream_conf**
-  * The status API is available at **127.0.0.1:8080/status**
+* We define a second virtual server listening on port 8080 and configure the NGINX Plus API on it, which is required by nginx-asg-sync:
+  * The API is available at **127.0.0.1:8080/api/2/**


127.0.0.1:8080/api/2/ -> 127.0.0.1:8080/api

pleshakov · 2018-06-22T17:15:31Z

README.md

-
-    location /upstream_conf {
-        upstream_conf;
+    location = /api {


location = /api -> location /api

also, add

location = /dashboard.html { root /usr/share/nginx/html; }

pleshakov · 2018-06-22T17:15:34Z

README.md

@@ -33,8 +33,7 @@ nginx-asg-sync uses the AWS API to get the list of IP addresses of the instances

 1. [Create an IAM role](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/iam-roles-for-amazon-ec2.html) and attach the predefined `AmazonEC2ReadOnlyAccess` policy to it. This policy allows read-only access to EC2 APIs.
 1. When you launch the NGINX Plus instance, add this IAM role to the instance.
-
-Alternatively, you can use the `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` environmental variables to provide credentials to nginx-asg-sync.
+1. Set `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` environment variables to provide credentials to nginx-asg-sync.


remove this line

pleshakov · 2018-06-22T17:16:02Z

cmd/sync/config_test.go

@@ -3,8 +3,7 @@ package main
 import "testing"

 var validYaml = []byte(`region: us-west-2
-upstream_conf_endpoint: http://127.0.0.1:8080/upstream_conf
-status_endpoint: http://127.0.0.1:8080/status
+api_endpoint: http://127.0.0.1:8080/api/


remove trailing /

pleshakov · 2018-06-22T17:16:16Z

cmd/sync/config_test.go

@@ -33,8 +32,7 @@ func getValidConfig() *config {
 	}
 	cfg := config{
 		Region:                "us-west-2",
-		UpstreamConfEndpont:   "http://127.0.0.1:8080/upstream_conf",
-		StatusEndpoint:        "http://127.0.0.1:8080/status",
+		APIEndpoint:           "http://127.0.0.1:8080/api/",


remove trailing /

pleshakov · 2018-06-22T17:21:24Z

cmd/sync/main.go

 			if err != nil {
 				log.Printf("Couldn't update servers in NGINX: %v", err)
 				continue
 			}
-			if len(removed) > 0 || len(added) > 0 {


bring this back :)

cmd/sync/plus/nginx_api.go

+)
+
+// NginxAPIController stores a NginxClient.
+type NginxAPIController struct {


pleshakov · 2018-06-22T17:27:07Z

cmd/sync/plus/nginx_api.go

+// HTTP Servers that are in the slice, but don't exist in NGINX, will be added to NGINX.
+// HTTP Servers that are not in the slice, but exist in NIGNX, will be removed from NGINX.
+func (nginx *NginxAPIController) UpdateServers(upstream string, servers []string, config ServerConfig) error {
+	var upsServers []UpstreamServer


move this logic - lines 34-41 into main
from main, call UpdateHTTPServers of NginxClient

also, set MaxFails to 1, as it is NGINX default

same applies to UpdateStreamServers

pleshakov · 2018-06-22T17:36:55Z

cmd/sync/plus/nginx_api.go

+	client *NginxClient
+}
+
+// ServerConfig stores a server configuration.


we don't need this type as well

we'll use NewNginxClient directly instead of NewNginxAPIController

pleshakov · 2018-06-22T17:39:14Z

cmd/sync/plus/nginx.go

@@ -0,0 +1,655 @@
+package plus


move this file back to cmd/sync

pleshakov

A few comments

pleshakov · 2018-06-25T14:58:31Z

README.md


 ### nginx-asg-sync Configuration

 nginx-asg-sync is configured in the file **aws.yaml** in the **/etc/nginx** folder. For our example, we define the following configuration:

 ```yaml
 region: us-west-2
-upstream_conf_endpoint: http://127.0.0.1:8080/upstream_conf
-status_endpoint: http://127.0.0.1:8080/status
+api_endpoint: http://127.0.0.1:8080/api/


remove the trailing /

pleshakov · 2018-06-25T16:02:09Z

cmd/sync/main.go

-				added, removed, err = nginx.UpdateHTTPServers(ups.Name, backends)
+			if upstream.Kind == "http" {
+				var upsServers []UpstreamServer
+				for _, server := range backends {


instead of iterating over backends, could we iterate over ips. same for line 120. this way we can remove lines 94-98.

pleshakov

Added a few suggestions

cmd/sync/config_test.go

@@ -49,14 +47,6 @@ func getInvalidConfigInput() []*testInput {
 	invalidRegionCfg.Region = ""
 	input = append(input, &testInput{invalidRegionCfg, "invalid region"})

-	invalidUpstreamConfEndponCfg := getValidConfig()


pleshakov · 2018-06-26T12:45:35Z

cmd/sync/nginx.go

 )

-// NginxClient lets you update HTTP/Stream servers in NGINX Plus via its upstream_conf API
+// Client version - 380a939


Please double check that it is 380a939

* Update to use unified /api endpoint * Update documentation with architecture * Update NGINX Go SDK * Update Go version * Add api invalid endpoint test

Dean-Coakley · 2018-07-26T21:20:57Z

Fixes: #5

Dean-Coakley self-assigned this Jun 15, 2018

Dean-Coakley requested a review from pleshakov June 15, 2018 10:13

pleshakov suggested changes Jun 22, 2018

View reviewed changes

Dean-Coakley added the enhancement Pull requests for new features/feature enhancements label Jun 25, 2018

Dean-Coakley requested a review from isaachawley June 25, 2018 14:36

isaachawley approved these changes Jun 25, 2018

View reviewed changes

pleshakov suggested changes Jun 25, 2018

View reviewed changes

Dean-Coakley force-pushed the nginx-plus-sdk-update branch 3 times, most recently from bfcabe1 to 2958d28 Compare June 26, 2018 11:56

pleshakov changed the title ~~Nginx Plus sdk update~~ Use new NGINX Plus API Jun 26, 2018

pleshakov suggested changes Jun 26, 2018

View reviewed changes

Use new NGINX Plus API

300cc70

* Update to use unified /api endpoint * Update documentation with architecture * Update NGINX Go SDK * Update Go version * Add api invalid endpoint test

Dean-Coakley force-pushed the nginx-plus-sdk-update branch from 2958d28 to 300cc70 Compare June 26, 2018 13:19

pleshakov merged commit b750af3 into master Jun 26, 2018

Dean-Coakley deleted the nginx-plus-sdk-update branch June 26, 2018 15:31

Dean-Coakley mentioned this pull request Aug 2, 2018

Move from status to api module #5

Closed

Use new NGINX Plus API #7

Use new NGINX Plus API #7

Uh oh!

Conversation

Dean-Coakley commented Jun 15, 2018

Uh oh!

pleshakov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pleshakov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pleshakov left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dean-Coakley commented Jul 26, 2018

Uh oh!

Uh oh!