APIGOV-26363 - fix several issues related to proxy mode discovery (#149)

* ISSUE-143 - skip publishing of api when no spec found

* ISSUE-140 - updates for deprovisioning access requests

* ISSUE-146 in use a mutex before writing to or reading spec map

* ISSUE-142 - gen url from bundle proxy config file

* code review comments

client/pkg/apigee/models/model_ssl_info.go

@@ -18,7 +18,7 @@ type SslInfo struct {
 	// Flag that specifies whether to enable one-way TLS/SSL. You must have defined a keystore containing the cert and private key.  **For Edge for Public Cloud**:  * You must have a cert signed by a trusted entity, such as Symantec or VeriSign. You cannot use a self-signed cert, or leaf certificates signed by a self-signed CA. * If your existing virtual host is configured to use a port other than `443`, you cannot change the TLS setting. That means you cannot change the TLS setting from enabled to disabled, or from disabled to enabled.
 	Enabled string `json:"enabled,omitempty"`
 	// Flag that specifies whether to ignore TLS certificate errors. This is similar to the `-k` option to curl.  This option is valid when configuring TLS for Target Servers and Target Endpoints, and when configuring virtual hosts that use 2-way TLS.  When used with a target endpoint/target server, if the backend system uses SNI and returns a cert with a subject Distinguished Name (DN) that does not match the hostname, there is no way to ignore the error and the connection fails.
-	IgnoreValidationErrors string `json:"ignoreValidationErrors,omitempty"`
+	IgnoreValidationErrors bool `json:"ignoreValidationErrors,omitempty"`
 	// Alias specified when you uploaded the cert and private key to the keystore. You must specify the alias name literally; you cannot use a reference. See <a href=\"https://docs.apigee.com/api-platform/system-administration/options-configuring-tls\">Options for configuring TLS</a> for more.
 	KeyAlias string `json:"keyAlias,omitempty"`
 	// Name of the keystore on Edge. Apigee recommends that you use a reference to specify the keystore name so that you can change the keystore without having to restart Routers. See <a href=\"https://docs.apigee.com/api-platform/system-administration/options-configuring-tls\">Options for configuring TLS</a> for more.

client/pkg/apigee/proxy.go

@@ -1,13 +1,30 @@
 package apigee
 
 import (
+	"archive/zip"
+	"bytes"
 	"encoding/json"
+	"encoding/xml"
 	"fmt"
+	"io"
 	"net/http"
 
 	"github.com/Axway/agents-apigee/client/pkg/apigee/models"
 )
 
+// proxyXML
+type proxyXML struct {
+	XMLName             xml.Name             `xml:"ProxyEndpoint"`
+	HTTPProxyConnection *HTTPProxyConnection `xml:"HTTPProxyConnection"`
+}
+
+// HTTPProxyConnection
+type HTTPProxyConnection struct {
+	XMLName     xml.Name `xml:"HTTPProxyConnection"`
+	BasePath    string   `xml:"BasePath"`
+	VirtualHost string   `xml:"VirtualHost"`
+}
+
 // Products
 type Proxies []string
 
@@ -47,7 +64,7 @@ func (a *ApigeeClient) GetProxy(proxyName string) (*models.ApiProxy, error) {
 	return proxy, nil
 }
 
-// GetProxy - get a proxy with a name
+// GetRevision - get a revision of a proxy with a name
 func (a *ApigeeClient) GetRevision(proxyName, revision string) (*models.ApiProxyRevision, error) {
 	response, err := a.newRequest(http.MethodGet, fmt.Sprintf("%s/apis/%s/revisions/%s", a.orgURL, proxyName, revision),
 		WithDefaultHeaders(),
@@ -65,6 +82,54 @@ func (a *ApigeeClient) GetRevision(proxyName, revision string) (*models.ApiProxy
 	return proxyRevision, nil
 }
 
+// GetRevisionConnectionType - get a revision bundle and open the proxy config file
+func (a *ApigeeClient) GetRevisionConnectionType(proxyName, revision string) (*HTTPProxyConnection, error) {
+	response, err := a.newRequest(http.MethodGet, fmt.Sprintf("%s/apis/%s/revisions/%s", a.orgURL, proxyName, revision),
+		WithDefaultHeaders(),
+		WithQueryParam("format", "bundle"),
+	).Execute()
+	if err != nil {
+		return nil, err
+	}
+
+	// response is a zip file, lets open it and find the proxy config file
+	zipReader, err := zip.NewReader(bytes.NewReader(response.Body), int64(len(response.Body)))
+	if err != nil {
+		return nil, err
+	}
+
+	// Read all the files from the zip archive
+	var fileBytes []byte
+	for _, zipFile := range zipReader.File {
+		if zipFile.Name != "apiproxy/proxies/default.xml" {
+			continue
+		}
+		fileBytes, err = readZipFile(zipFile)
+		if err != nil {
+			return nil, err
+		}
+		break
+	}
+
+	if len(fileBytes) == 0 {
+		return nil, fmt.Errorf("could not find the proxy configuration file in the api revision bundle")
+	}
+
+	data := &proxyXML{}
+	xml.Unmarshal(fileBytes, data)
+
+	return data.HTTPProxyConnection, nil
+}
+
+func readZipFile(zf *zip.File) ([]byte, error) {
+	f, err := zf.Open()
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	return io.ReadAll(f)
+}
+
 // GetProxy - get a proxy with a name
 func (a *ApigeeClient) GetRevisionResourceFile(proxyName, revision, resourceType, resourceName string) ([]byte, error) {
 	response, err := a.newRequest(http.MethodGet, fmt.Sprintf("%s/apis/%s/revisions/%s/resourcefiles/%s/%s", a.orgURL, proxyName, revision, resourceType, resourceName),

client/scripts/apigee_generate.sh

@@ -22,5 +22,6 @@ sed -i -r 's/Timestamp.string/Timestamp int64/g' /codegen/output/model_metrics_v
 # replace the model_metrics_metrics.go file with the template for the custom unmarshal
 cp ./model_metrics_metrics.tmpl /codegen/output/model_metrics_metrics.go
 cp ./model_virtual_host.tmpl /codegen/output/model_virtual_host.go
+cp ./model_ssl_info.tmpl /codegen/output/model_ssl_info.go
 
 rm ./openapitools.json

client/scripts/model_ssl_info.tmpl

@@ -0,0 +1,22 @@
+// Modified after generation
+package models
+
+// SslInfo SSL information.
+type SslInfo struct {
+	// **Edge for Private Cloud version 4.15.07 and earlier only.**  Specifies the ciphers supported by the virtual host. If no ciphers are specified, then all ciphers available for the JVM will be permitted.  To restrict ciphers, add the following elements: `TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA` and `TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256`
+	Ciphers []string `json:"ciphers,omitempty"`
+	// Flag that specifies whether to enable two-way, or client, TLS between Edge (server) and the app (client) making the request. Enabling two-way TLS requires that you set up a truststore on Edge that contains the cert from the TLS client.
+	ClientAuthEnabled string `json:"clientAuthEnabled,omitempty"`
+	// Flag that specifies whether to enable one-way TLS/SSL. You must have defined a keystore containing the cert and private key.  **For Edge for Public Cloud**:  * You must have a cert signed by a trusted entity, such as Symantec or VeriSign. You cannot use a self-signed cert, or leaf certificates signed by a self-signed CA. * If your existing virtual host is configured to use a port other than `443`, you cannot change the TLS setting. That means you cannot change the TLS setting from enabled to disabled, or from disabled to enabled.
+	Enabled string `json:"enabled,omitempty"`
+	// Flag that specifies whether to ignore TLS certificate errors. This is similar to the `-k` option to curl.  This option is valid when configuring TLS for Target Servers and Target Endpoints, and when configuring virtual hosts that use 2-way TLS.  When used with a target endpoint/target server, if the backend system uses SNI and returns a cert with a subject Distinguished Name (DN) that does not match the hostname, there is no way to ignore the error and the connection fails.
+	IgnoreValidationErrors bool `json:"ignoreValidationErrors,omitempty"`
+	// Alias specified when you uploaded the cert and private key to the keystore. You must specify the alias name literally; you cannot use a reference. See <a href=\"https://docs.apigee.com/api-platform/system-administration/options-configuring-tls\">Options for configuring TLS</a> for more.
+	KeyAlias string `json:"keyAlias,omitempty"`
+	// Name of the keystore on Edge. Apigee recommends that you use a reference to specify the keystore name so that you can change the keystore without having to restart Routers. See <a href=\"https://docs.apigee.com/api-platform/system-administration/options-configuring-tls\">Options for configuring TLS</a> for more.
+	KeyStore string `json:"keyStore,omitempty"`
+	// **Edge for Private Cloud version 4.15.07 and earlier only.**  Specifies the protocols supported by the virtual host. If no protocols are specified, then all protocols available for the JVM will be permitted.  To restrict protocols, add the following elements: `TLSv1`, `TLSv1.2`, and `SSLv2Hello`
+	Protocols []string `json:"protocols,omitempty"`
+	// Name of the truststore on Edge that contains the certificate or certificate chain used for two-way TLS. Required if `clientAuthEnabled` is `true`.  Apigee recommends that you use a reference to specify the truststore name so that you can change the truststore without having to restart Routers. See <a href=\"https://docs.apigee.com/api-platform/system-administration/options-configuring-tls\">Options for configuring TLS</a> for more.
+	TrustStore string `json:"trustStore,omitempty"`
+}

discovery/pkg/apigee/cache.go

@@ -3,6 +3,7 @@ package apigee
 import (
 	"fmt"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/Axway/agent-sdk/pkg/apic"
@@ -12,6 +13,7 @@ import (
 type agentCache struct {
 	cache              cache.Cache
 	specEndpointToKeys map[string][]specCacheItem
+	mutex              *sync.Mutex
 }
 
 type specCacheItem struct {
@@ -25,6 +27,7 @@ func newAgentCache() *agentCache {
 	return &agentCache{
 		cache:              cache.New(),
 		specEndpointToKeys: make(map[string][]specCacheItem),
+		mutex:              &sync.Mutex{},
 	}
 }
 
@@ -43,6 +46,8 @@ func (a *agentCache) AddSpecToCache(id, path, name string, modDate time.Time, en
 	a.cache.SetWithSecondaryKey(specPrimaryKey(name), path, item)
 	a.cache.SetSecondaryKey(specPrimaryKey(name), strings.ToLower(name))
 	a.cache.SetSecondaryKey(specPrimaryKey(name), id)
+	a.mutex.Lock()
+	defer a.mutex.Unlock()
 	for _, ep := range endpoints {
 		if _, found := a.specEndpointToKeys[ep]; !found {
 			a.specEndpointToKeys[ep] = []specCacheItem{}
@@ -90,6 +95,8 @@ func (a *agentCache) GetSpecWithName(name string) (*specCacheItem, error) {
 
 // GetSpecPathWithEndpoint - returns the lat modified spec found with this endpoint
 func (a *agentCache) GetSpecPathWithEndpoint(endpoint string) (string, error) {
+	a.mutex.Lock()
+	defer a.mutex.Unlock()
 	items, found := a.specEndpointToKeys[endpoint]
 	if !found {
 		return "", fmt.Errorf("no spec found for endpoint: %s", endpoint)

discovery/pkg/apigee/pollproxiesjob.go

@@ -35,6 +35,7 @@ type proxyClient interface {
 	GetAllProxies() (apigee.Proxies, error)
 	GetRevision(proxyName, revision string) (*models.ApiProxyRevision, error)
 	GetRevisionResourceFile(proxyName, revision, resourceType, resourceName string) ([]byte, error)
+	GetRevisionConnectionType(proxyName, revision string) (*apigee.HTTPProxyConnection, error)
 	GetDeployments(apiName string) (*models.DeploymentDetails, error)
 	GetVirtualHost(envName, virtualHostName string) (*models.VirtualHost, error)
 	GetSpecFile(specPath string) ([]byte, error)
@@ -52,28 +53,32 @@ type proxyCache interface {
 // job that will poll for any new portals on APIGEE Edge
 type pollProxiesJob struct {
 	jobs.Job
-	client      proxyClient
-	cache       proxyCache
-	firstRun    bool
-	logger      log.FieldLogger
-	specsReady  jobFirstRunDone
-	pubLock     sync.Mutex
-	publishFunc agent.PublishAPIFunc
-	workers     int
-	running     bool
-	runningLock sync.Mutex
+	client          proxyClient
+	cache           proxyCache
+	firstRun        bool
+	logger          log.FieldLogger
+	specsReady      jobFirstRunDone
+	pubLock         sync.Mutex
+	publishFunc     agent.PublishAPIFunc
+	workers         int
+	running         bool
+	runningLock     sync.Mutex
+	virtualHostURLs map[string]map[string][]string
+	lastTime        int
+	runTime         int
 }
 
 func newPollProxiesJob(client proxyClient, cache proxyCache, specsReady jobFirstRunDone, workers int) *pollProxiesJob {
 	job := &pollProxiesJob{
-		client:      client,
-		cache:       cache,
-		firstRun:    true,
-		specsReady:  specsReady,
-		logger:      log.NewFieldLogger().WithComponent("pollProxies").WithPackage("apigee"),
-		publishFunc: agent.PublishAPI,
-		workers:     workers,
-		runningLock: sync.Mutex{},
+		client:          client,
+		cache:           cache,
+		firstRun:        true,
+		specsReady:      specsReady,
+		logger:          log.NewFieldLogger().WithComponent("pollProxies").WithPackage("apigee"),
+		publishFunc:     agent.PublishAPI,
+		workers:         workers,
+		runningLock:     sync.Mutex{},
+		virtualHostURLs: make(map[string]map[string][]string),
 	}
 	return job
 }
@@ -131,6 +136,7 @@ func (j *pollProxiesJob) Execute() error {
 
 	wg := sync.WaitGroup{}
 	wg.Add(len(allProxies))
+	j.runTime = j.lastTime
 	for _, proxyName := range allProxies {
 		go func() {
 			defer wg.Done()
@@ -202,6 +208,13 @@ func (j *pollProxiesJob) handleRevision(ctx context.Context, revName string) {
 		return
 	}
 
+	if revision.LastModifiedAt <= j.runTime {
+		return
+	}
+	if j.lastTime < revision.LastModifiedAt {
+		j.lastTime = revision.LastModifiedAt
+	}
+
 	ctx = context.WithValue(ctx, revNameField, revision)
 	logger = logger.WithField(revNameField.String(), revision.Revision)
 	addLoggerToContext(ctx, logger)
@@ -275,18 +288,29 @@ func (j *pollProxiesJob) getVirtualHostURLs(ctx context.Context) context.Context
 	revision := ctx.Value(revNameField).(*models.ApiProxyRevision)
 	envName := getStringFromContext(ctx, envNameField)
 	proxyName := getStringFromContext(ctx, proxyNameField)
-
-	// attempt to get the spec from the endpoints the revision is hosted on
 	allURLs := []string{}
-	for _, virtualHostName := range revision.Proxies {
-		logger := logger.WithField("virtualHostName", virtualHostName)
-		virtualHost, err := j.client.GetVirtualHost(envName, virtualHostName)
+
+	connection, err := j.client.GetRevisionConnectionType(proxyName, revision.Revision)
+	if err != nil {
+		logger.WithError(err).Error("could not get the revision connection type")
+		return context.WithValue(ctx, endpointsField, allURLs)
+	}
+
+	if _, ok := j.virtualHostURLs[envName]; !ok {
+		j.virtualHostURLs[envName] = make(map[string][]string)
+	}
+
+	if _, ok := j.virtualHostURLs[envName][connection.VirtualHost]; !ok {
+		virtualHost, err := j.client.GetVirtualHost(envName, connection.VirtualHost)
 		if err != nil {
-			logger.WithError(err).Debug("could not get virtual host details")
-			continue
+			logger.WithError(err).Error("could not get the virtual host info")
+			return context.WithValue(ctx, endpointsField, allURLs)
 		}
-		urls := urlsFromVirtualHost(virtualHost, proxyName)
-		allURLs = append(allURLs, urls...)
+		j.virtualHostURLs[envName][connection.VirtualHost] = urlsFromVirtualHost(virtualHost)
+	}
+
+	for _, url := range j.virtualHostURLs[envName][connection.VirtualHost] {
+		allURLs = append(allURLs, fmt.Sprintf("%s%s", url, connection.BasePath))
 	}
 
 	return context.WithValue(ctx, endpointsField, allURLs)
@@ -396,7 +420,7 @@ func (j *pollProxiesJob) buildServiceBody(ctx context.Context) (*apic.ServiceBod
 	}
 
 	if len(spec) == 0 {
-		logger.Debug("creating without a spec")
+		return nil, fmt.Errorf("skipping proxy creation without a spec")
 	}
 	logger.Debug("creating service body")
 

discovery/pkg/apigee/pollproxiesjob_test.go

@@ -57,11 +57,10 @@ func Test_pollProxiesJob(t *testing.T) {
 			hasOauth: true,
 		},
 		{
-			name:      "should create proxy when no spec found but has api key policy",
-			hasAPIKey: true,
+			name: "should create proxy when no spec found",
 		},
 		{
-			name: "should create proxy when no spec found",
+			name: "should stop when no spec found but has api key policy",
 		},
 		{
 			name:           "should stop when getting proxy revision fails",
@@ -206,6 +205,10 @@ func (m mockProxyClient) GetRevision(apiName, revision string) (rev *models.ApiP
 	return
 }
 
+func (m mockProxyClient) GetRevisionConnectionType(proxyName, revision string) (*apigee.HTTPProxyConnection, error) {
+	return nil, nil
+}
+
 func (m mockProxyClient) GetRevisionResourceFile(apiName, revision, resourceType, resourceName string) ([]byte, error) {
 	assert.Contains(m.t, proxyName, apiName)
 	assert.Contains(m.t, revName, revision)