This repository has been archived by the owner on Mar 11, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 86
[WIP] migrate WIT to use proxy for all deployments API requests #2284
Open
stooke
wants to merge
33
commits into
fabric8-services:master
Choose a base branch
from
stooke:jst-fullproxy
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 31 commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
b6fcaba
Eliminate some more duplicate names for deployments API refactoring
stooke 36e590a
Merge remote-tracking branch 'upstream/master'
stooke 58342c7
Merge remote-tracking branch 'upstream/master'
stooke a45161a
Merge remote-tracking branch 'upstream/master'
stooke dbcf947
Merge remote-tracking branch 'upstream/master'
stooke 99d3f01
remove unused /apps API
stooke e45ac78
Merge remote-tracking branch 'upstream/master'
stooke 2b06816
Merge remote-tracking branch 'upstream/master'
stooke 94159dc
Merge remote-tracking branch 'upstream/master'
stooke 9bb38c8
Merge remote-tracking branch 'upstream/master'
stooke ad3b87f
Merge remote-tracking branch 'upstream/master'
stooke e32a323
Merge remote-tracking branch 'upstream/master'
stooke 1a9481d
Merge remote-tracking branch 'upstream/master'
stooke ff47976
Merge remote-tracking branch 'upstream/master'
stooke 29461ed
Merge remote-tracking branch 'upstream/master'
stooke 15ee913
Merge remote-tracking branch 'upstream/master'
stooke 7df6871
Merge remote-tracking branch 'upstream/master'
stooke f9a252e
Merge remote-tracking branch 'upstream/master'
stooke 99f3d57
Merge remote-tracking branch 'upstream/master'
stooke 8193a2f
Merge remote-tracking branch 'upstream/master'
stooke 7ebe5eb
Merge remote-tracking branch 'upstream/master'
stooke d1514f0
Merge remote-tracking branch 'upstream/master'
stooke d2eb5e3
Merge remote-tracking branch 'upstream/master'
stooke 63c3ec9
Merge remote-tracking branch 'upstream/master'
stooke fa0d3bd
Merge remote-tracking branch 'upstream/master'
stooke 2418c94
Merge remote-tracking branch 'upstream/master'
stooke 7a7c9e8
Merge remote-tracking branch 'upstream/master'
stooke e4c3a6c
Merge remote-tracking branch 'upstream/master'
stooke ee6062b
simplify URL provider to only use proxy service
stooke 32e319f
Merge remote-tracking branch 'upstream/master' into jst-fullproxy
stooke 9d8c3c1
Merge remote-tracking branch 'upstream/master' into jst-fullproxy
stooke 9709d60
Merge remote-tracking branch 'upstream/master' into jst-fullproxy
stooke 60a85f1
remove references to Tenant service
stooke File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,86 +2,23 @@ package controller | |
|
||
import ( | ||
"context" | ||
"encoding/json" | ||
"fmt" | ||
"io/ioutil" | ||
"net/http" | ||
"net/url" | ||
"strings" | ||
|
||
"github.com/fabric8-services/fabric8-wit/app" | ||
"github.com/fabric8-services/fabric8-wit/auth" | ||
"github.com/fabric8-services/fabric8-wit/auth/authservice" | ||
"github.com/fabric8-services/fabric8-wit/configuration" | ||
"github.com/fabric8-services/fabric8-wit/goasupport" | ||
"github.com/fabric8-services/fabric8-wit/kubernetes" | ||
"github.com/fabric8-services/fabric8-wit/log" | ||
|
||
goajwt "github.com/goadesign/goa/middleware/security/jwt" | ||
errs "github.com/pkg/errors" | ||
) | ||
|
||
// there are several concrete instantiations: | ||
// | ||
// - access /api/user/services instead of Auth | ||
// - use proxy (if present) for normal OSO API calls | ||
// - access OSO metrics directly (until proxy supports this) | ||
// This incarnation uses the proxy for ALL OSO API calls and will not function without a proxy | ||
|
||
type tenantURLProvider struct { | ||
apiURL string | ||
apiToken string | ||
tenant *app.UserService | ||
namespaces map[string]*app.NamespaceAttributes | ||
tokens map[string]string | ||
TokenRetriever | ||
} | ||
|
||
// TokenRetriever is a service that will respond with an authorization token, given a service endpoint or name | ||
type TokenRetriever interface { | ||
TokenForService(serviceURL string) (*string, error) | ||
} | ||
|
||
type tokenRetriever struct { | ||
authClient *authservice.Client | ||
context context.Context | ||
} | ||
|
||
func (tr *tokenRetriever) TokenForService(forService string) (*string, error) { | ||
|
||
resp, err := tr.authClient.RetrieveToken(goasupport.ForwardContextRequestID(tr.context), authservice.RetrieveTokenPath(), forService, nil) | ||
if err != nil { | ||
return nil, errs.Wrapf(err, "unable to retrieve Auth token for '%s' service", forService) | ||
} | ||
|
||
defer resp.Body.Close() | ||
|
||
respBody, err := ioutil.ReadAll(resp.Body) | ||
|
||
status := resp.StatusCode | ||
if status != http.StatusOK { | ||
log.Error(nil, map[string]interface{}{ | ||
"err": err, | ||
"request_path": authservice.ShowUserPath(), | ||
"for_service": forService, | ||
"http_status": status, | ||
}, "failed to GET token from auth service due to HTTP error %s", status) | ||
return nil, errs.Errorf("failed to GET Auth token for '%s' service due to status code %d", forService, status) | ||
} | ||
|
||
var respType authservice.TokenData | ||
err = json.Unmarshal(respBody, &respType) | ||
if err != nil { | ||
log.Error(nil, map[string]interface{}{ | ||
"err": err, | ||
"request_path": authservice.ShowUserPath(), | ||
"for_service": forService, | ||
"http_status": status, | ||
"response_body": respBody, | ||
}, "unable to unmarshal Auth token") | ||
return nil, errs.Wrapf(err, "unable to unmarshal Auth token for '%s' service from Auth service", forService) | ||
} | ||
|
||
return respType.AccessToken, nil | ||
apiURL string | ||
apiToken string | ||
kubernetes.BaseURLProvider | ||
} | ||
|
||
// ensure tenantURLProvider implements BaseURLProvider | ||
|
@@ -91,130 +28,31 @@ var _ kubernetes.BaseURLProvider = (*tenantURLProvider)(nil) | |
// NewURLProvider looks at what servers are available and create a BaseURLProvder that fits | ||
func NewURLProvider(ctx context.Context, config *configuration.Registry, osioclient OpenshiftIOClient) (kubernetes.BaseURLProvider, error) { | ||
|
||
userServices, err := osioclient.GetUserServices(ctx) | ||
if err != nil { | ||
log.Error(ctx, map[string]interface{}{ | ||
"err": err, | ||
}, "error accessing Tenant API") | ||
return nil, err | ||
} | ||
|
||
token := goajwt.ContextJWT(ctx).Raw | ||
proxyURL := config.GetOpenshiftProxyURL() | ||
|
||
up, err := newTenantURLProviderFromTenant(userServices, token, proxyURL) | ||
if err != nil { | ||
return nil, err | ||
if len(proxyURL) == 0 { | ||
log.Error(ctx, map[string]interface{}{}, "No Proxy URL configured") | ||
return nil, errs.Errorf("No Proxy URL configured") | ||
} | ||
|
||
// create Auth API client - required to get OSO tokens | ||
authClient, err := auth.CreateClient(ctx, config) | ||
up, err := NewProxyURLProvider(token, proxyURL) | ||
if err != nil { | ||
log.Error(ctx, map[string]interface{}{ | ||
"err": err, | ||
}, "error accessing Auth server") | ||
return nil, errs.Wrap(err, "error creating Auth client") | ||
} | ||
up.TokenRetriever = &tokenRetriever{ | ||
authClient: authClient, | ||
context: ctx, | ||
return nil, err | ||
} | ||
|
||
// if we're not using a proxy then the API URL is actually the cluster of namespace 0, | ||
// so the apiToken should be the token for that cluster. | ||
// there should be no defaults, but that's deferred later | ||
if len(proxyURL) == 0 { | ||
tokenData, err := up.TokenForService(up.apiURL) | ||
if err != nil { | ||
return nil, err | ||
} | ||
up.apiToken = *tokenData | ||
} | ||
return up, nil | ||
} | ||
|
||
// newTenantURLProviderFromTenant create a provider from a UserService object | ||
func newTenantURLProviderFromTenant(t *app.UserService, token string, proxyURL string) (*tenantURLProvider, error) { | ||
|
||
if t.ID == nil { | ||
log.Error(nil, map[string]interface{}{}, "app.UserService is malformed: no ID field") | ||
return nil, errs.New("app.UserService is malformed: no ID field") | ||
} | ||
|
||
if t.Attributes == nil { | ||
log.Error(nil, map[string]interface{}{ | ||
"tenant": *t.ID, | ||
}, "app.UserService is malformed: no Attribute field ID=%s", *t.ID) | ||
return nil, errs.Errorf("app.UserService is malformed: no Attribute field (ID=%s)", *t.ID) | ||
} | ||
|
||
if len(t.Attributes.Namespaces) == 0 { | ||
log.Error(nil, map[string]interface{}{ | ||
"tenant": *t.ID, | ||
}, "this tenant has no namespaces: %s", *t.ID) | ||
return nil, errs.Errorf("app.UserService is malformed: no Namespaces (ID=%s)", *t.ID) | ||
} | ||
|
||
defaultNamespace := t.Attributes.Namespaces[0] | ||
namespaceMap := make(map[string]*app.NamespaceAttributes) | ||
for i, namespace := range t.Attributes.Namespaces { | ||
namespaceMap[*namespace.Name] = t.Attributes.Namespaces[i] | ||
if namespace.Type != nil && *namespace.Type == "user" { | ||
defaultNamespace = namespace | ||
} | ||
} | ||
|
||
defaultClusterURL := *defaultNamespace.ClusterURL | ||
|
||
if len(proxyURL) != 0 { | ||
// all non-metric API calls go via the proxy | ||
defaultClusterURL = proxyURL | ||
} | ||
|
||
// NewProxyURLProvider create a provider from a UserService object (exposed for testing) | ||
func NewProxyURLProvider(token string, proxyURL string) (kubernetes.BaseURLProvider, error) { | ||
provider := &tenantURLProvider{ | ||
apiURL: defaultClusterURL, | ||
apiToken: token, | ||
tenant: t, | ||
namespaces: namespaceMap, | ||
apiURL: proxyURL, | ||
apiToken: token, | ||
} | ||
return provider, nil | ||
} | ||
|
||
// NewTenantURLProviderFromTenant create a provider from a UserService object (exposed for testing) | ||
func NewTenantURLProviderFromTenant(t *app.UserService, token string, proxyURL string) (kubernetes.BaseURLProvider, error) { | ||
return newTenantURLProviderFromTenant(t, token, proxyURL) | ||
} | ||
|
||
// GetEnvironmentMapping returns a map whose keys are environment names, and values are the Kubernetes namespaces | ||
// that represent those environments | ||
func (up *tenantURLProvider) GetEnvironmentMapping() map[string]string { | ||
result := make(map[string]string) | ||
// Exclude internal namespaces where the user cannot deploy applications | ||
|
||
// Deployments API will receive requests by environment name (e.g. "run", "stage"). | ||
// These names correspond to the "type" attribute in Namespaces. | ||
for envNS, attr := range up.namespaces { | ||
envName := attr.Type | ||
if envName == nil || len(*envName) == 0 { | ||
log.Error(nil, map[string]interface{}{ | ||
"namespace": envNS, | ||
}, "namespace has no type") | ||
} else { | ||
result[*envName] = envNS | ||
} | ||
} | ||
return result | ||
} | ||
|
||
// Types of namespaces where the user does not deploy applications | ||
var internalNamespaceTypes = map[string]struct{}{"user": {}, "che": {}, "jenkins": {}} | ||
|
||
// CanDeploy returns true if the environment type provided can be deployed to as part of a pipeline | ||
func (up *tenantURLProvider) CanDeploy(envType string) bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need this one too, to filter out environments we want to hide like che and jenkins. |
||
_, pres := internalNamespaceTypes[envType] | ||
return !pres | ||
} | ||
|
||
func (up *tenantURLProvider) GetAPIToken() (*string, error) { | ||
return &up.apiToken, nil | ||
} | ||
|
@@ -225,112 +63,54 @@ func (up *tenantURLProvider) GetAPIURL() (*string, error) { | |
} | ||
|
||
func (up *tenantURLProvider) GetMetricsToken(envNS string) (*string, error) { | ||
// since metrics bypasses the proxy, this is the OSO cluster token | ||
token := up.tokens[envNS] | ||
if len(token) == 0 { | ||
ns := up.namespaces[envNS] | ||
if ns == nil { | ||
return nil, errs.Errorf("Namespace '%s' is not in tenant '%s'", envNS, *up.tenant.ID) | ||
} | ||
if up.TokenRetriever != nil { | ||
tokenData, err := up.TokenForService(*ns.ClusterURL) | ||
if err != nil { | ||
return nil, err | ||
} | ||
token = *tokenData | ||
} else { | ||
tokenData, err := up.GetAPIToken() | ||
if err != nil { | ||
return nil, err | ||
} | ||
token = *tokenData | ||
} | ||
} | ||
return &token, nil | ||
return &up.apiToken, nil | ||
} | ||
|
||
func (up *tenantURLProvider) GetConsoleURL(envNS string) (*string, error) { | ||
ns := up.namespaces[envNS] | ||
if ns == nil { | ||
return nil, errs.Errorf("Namespace '%s' is not in tenant '%s'", envNS, *up.tenant.ID) | ||
} | ||
// Note that the Auth/Tenant appends /console to the hostname for console/logging | ||
baseURL := ns.ClusterConsoleURL | ||
if baseURL == nil || len(*baseURL) == 0 { | ||
// if it's missing, modify the cluster URL | ||
bu, err := modifyURL(*ns.ClusterURL, "console", "/console") | ||
if err != nil { | ||
return nil, err | ||
} | ||
buStr := bu.String() | ||
baseURL = &buStr | ||
mu, err := modifyPath(up.apiURL, "/console") | ||
if err != nil { | ||
return nil, err | ||
} | ||
consoleURL := fmt.Sprintf("%s/project/%s", *baseURL, envNS) | ||
urlStr := mu.String() | ||
|
||
consoleURL := fmt.Sprintf("%s/project/%s", urlStr, envNS) | ||
return &consoleURL, nil | ||
} | ||
|
||
func (up *tenantURLProvider) GetLoggingURL(envNS string, deployName string) (*string, error) { | ||
ns := up.namespaces[envNS] | ||
if ns == nil { | ||
return nil, errs.Errorf("Namespace '%s' is not in tenant '%s'", envNS, *up.tenant.ID) | ||
} | ||
// Note that the Auth/Tenant appends /console to the hostname for console/logging | ||
baseURL := ns.ClusterLoggingURL | ||
if baseURL == nil || len(*baseURL) == 0 { | ||
// if it's missing, modify the cluster URL | ||
bu, err := modifyURL(*ns.ClusterURL, "console", "/console") | ||
if err != nil { | ||
return nil, err | ||
} | ||
buStr := bu.String() | ||
baseURL = &buStr | ||
mu, err := modifyPath(up.apiURL, "/logs") | ||
if err != nil { | ||
return nil, err | ||
} | ||
loggingURL := fmt.Sprintf("%s/project/%s/browse/rc/%s?tab=logs", *baseURL, envNS, deployName) | ||
urlStr := mu.String() | ||
|
||
loggingURL := fmt.Sprintf("%s/project/%s/browse/rc/%s?tab=logs", urlStr, envNS, deployName) | ||
return &loggingURL, nil | ||
} | ||
|
||
func (up *tenantURLProvider) GetMetricsURL(envNS string) (*string, error) { | ||
ns := up.namespaces[envNS] | ||
if ns == nil { | ||
return nil, errs.Errorf("Namespace '%s' is not in tenant '%s'", envNS, *up.tenant.ID) | ||
} | ||
|
||
baseURL := ns.ClusterMetricsURL | ||
if baseURL == nil || len(*baseURL) == 0 { | ||
// In the absence of a better way (i.e. tenant) to get the user's metrics URL, | ||
// substitute "api" with "metrics" in user's cluster URL | ||
mu, err := modifyURL(*ns.ClusterURL, "metrics", "") | ||
if err != nil { | ||
return nil, err | ||
} | ||
muStr := mu.String() | ||
baseURL = &muStr | ||
} | ||
// Hawkular implementation is sensitive and requires no trailing '/' | ||
if strings.HasSuffix(*baseURL, "/") { | ||
nurl := (*baseURL)[:len(*baseURL)-1] | ||
baseURL = &nurl | ||
// substitute "api" with "metrics" in user's cluster URL | ||
mu, err := modifyPath(up.apiURL, "/metrics") | ||
if err != nil { | ||
return nil, err | ||
} | ||
return baseURL, nil | ||
urlStr := mu.String() | ||
|
||
return &urlStr, nil | ||
} | ||
|
||
func modifyURL(apiURLStr string, prefix string, path string) (*url.URL, error) { | ||
func modifyPath(apiURLStr string, path string) (*url.URL, error) { | ||
// Parse as URL to give us easy access to the hostname | ||
apiURL, err := url.Parse(apiURLStr) | ||
if err != nil { | ||
return nil, errs.WithStack(err) | ||
} | ||
|
||
// Get the hostname (without port) and replace api prefix with prefix arg | ||
apiHostname := apiURL.Hostname() | ||
if !strings.HasPrefix(apiHostname, "api") { | ||
return nil, errs.Errorf("cluster URL does not begin with \"api\": %s", apiHostname) | ||
} | ||
newHostname := strings.Replace(apiHostname, "api", prefix, 1) | ||
// Construct URL using just scheme from API URL, modified hostname and supplied path | ||
newURL := &url.URL{ | ||
Scheme: apiURL.Scheme, | ||
Host: newHostname, | ||
Host: apiURL.Hostname(), | ||
Path: path, | ||
} | ||
return newURL, nil | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will still need this method to translate environment names to Kubernetes namespaces using the information in Tenant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I was hoping to get rid of Tenant in our code, but it seems it's not possible.
I think this API as is is unusable because of the issues @ebaron pointed out, so I propose withdrawing it. The current code will work via the proxy if so configured; this was mainly a cleanup PR. A proper refactoring would leave most of this PR as I have it here (i.e. simplify the URLProvider code), but also add a TenantClient that provides the mapping we reuire from the Tenant service.