Expose UUID in API URLs #26
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
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
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
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
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
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 |
|---|---|---|
|
|
@@ -32,6 +32,7 @@ var ( | |
| type apiState interface { | ||
| Close() error | ||
| APIHostPorts() [][]instance.HostPort | ||
| EnvironTag() string | ||
| } | ||
|
|
||
| type apiOpenFunc func(*api.Info, api.DialOpts) (apiState, error) | ||
|
|
@@ -212,7 +213,7 @@ func newAPIFromStore(envName string, store configstore.Storage, apiOpen apiOpenF | |
|
|
||
| st := val0.(apiState) | ||
| // Even though we are about to update API addresses based on | ||
| // APIHostPorts in cacheChangedAPIInfo, we first cache the | ||
| // addresses based on the provider lookup. This is because older API | ||
| // servers didn't return their HostPort information on Login, and we | ||
| // still want to cache our connection information to them. | ||
|
|
@@ -231,7 +232,7 @@ func newAPIFromStore(envName string, store configstore.Storage, apiOpen apiOpenF | |
| } | ||
| } | ||
| // Update API addresses if they've changed. Error is non-fatal. | ||
| if localerr := cacheChangedAPIInfo(info, st); localerr != nil { | ||
| logger.Warningf("cannot failed to cache API addresses: %v", localerr) | ||
| } | ||
| return st, nil | ||
|
|
@@ -267,11 +268,18 @@ func apiInfoConnect(store configstore.Storage, info configstore.EnvironInfo, api | |
| return nil, &infoConnectError{fmt.Errorf("no cached addresses")} | ||
| } | ||
| logger.Infof("connecting to API addresses: %v", endpoint.Addresses) | ||
| environTag := "" | ||
| if endpoint.EnvironUUID != "" { | ||
| // Note: we should be validating that EnvironUUID contains a | ||
| // valid UUID. | ||
| environTag = names.EnvironTag(endpoint.EnvironUUID) | ||
|
There was a problem hiding this comment. We should check EnvironUUID contains a valid UUID, there is LP bug http://pad.lv/1257587 for fixing names.IsEnvironment() to be more strict. It's not something to do now, only perhaps add a TODO comment here referencing the same bug and some notice we need to improve this. There was a problem hiding this comment. I added a comment, though since the next steps are about comparing it with the concrete UUID someone else has, it is being validated, just not early. |
||
| } | ||
| apiInfo := &api.Info{ | ||
| Addrs: endpoint.Addresses, | ||
| CACert: endpoint.CACert, | ||
| Tag: names.UserTag(info.APICredentials().User), | ||
| Password: info.APICredentials().Password, | ||
| EnvironTag: environTag, | ||
| } | ||
| st, err := apiOpen(apiInfo, api.DefaultDialOpts()) | ||
| if err != nil { | ||
|
|
@@ -343,14 +351,24 @@ func environAPIInfo(environ environs.Environ) (*api.Info, error) { | |
| // cacheAPIInfo updates the local environment settings (.jenv file) | ||
| // with the provided apiInfo, assuming we've just successfully | ||
| // connected to the API server. | ||
| func cacheAPIInfo(info configstore.EnvironInfo, apiInfo *api.Info) (err error) { | ||
| defer errors.Contextf(&err, "failed to cache API credentials") | ||
| environUUID := "" | ||
| if apiInfo.EnvironTag != "" { | ||
| var err error | ||
| _, environUUID, err = names.ParseTag(apiInfo.Tag, names.EnvironTagKind) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
| info.SetAPIEndpoint(configstore.APIEndpoint{ | ||
| Addresses: apiInfo.Addrs, | ||
| CACert: string(apiInfo.CACert), | ||
| EnvironUUID: environUUID, | ||
| }) | ||
| _, username, err := names.ParseTag(apiInfo.Tag, names.UserTagKind) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| info.SetAPICredentials(configstore.APICredentials{ | ||
| User: username, | ||
|
|
@@ -359,9 +377,10 @@ func cacheAPIInfo(info configstore.EnvironInfo, apiInfo *api.Info) error { | |
| return info.Write() | ||
| } | ||
|
|
||
| // cacheChangedAPIInfo updates the local environment settings (.jenv file) | ||
| // with the provided API server addresses if they have changed. It will also | ||
| // save the environment tag if it is available. | ||
| func cacheChangedAPIInfo(info configstore.EnvironInfo, st apiState) error { | ||
| var addrs []string | ||
| for _, serverHostPorts := range st.APIHostPorts() { | ||
| for _, hostPort := range serverHostPorts { | ||
|
|
@@ -373,11 +392,23 @@ func cacheChangedAPIAddresses(info configstore.EnvironInfo, st apiState) error { | |
| } | ||
| } | ||
| endpoint := info.APIEndpoint() | ||
| newEnvironTag := st.EnvironTag() | ||
| changed := false | ||
| if newEnvironTag != "" { | ||
| _, environUUID, err := names.ParseTag(newEnvironTag, names.EnvironTagKind) | ||
|
There was a problem hiding this comment. Can/should environ UUID change at all like this? It feels like we should log it at least when it's known to have changed. There was a problem hiding this comment. Well it definitely can change from empty to containing something. |
||
| if err == nil && endpoint.EnvironUUID != environUUID { | ||
| changed = true | ||
| endpoint.EnvironUUID = environUUID | ||
| } | ||
| } | ||
| if len(addrs) != 0 && addrsChanged(endpoint.Addresses, addrs) { | ||
| logger.Debugf("API addresses changed from %q to %q", endpoint.Addresses, addrs) | ||
| changed = true | ||
| endpoint.Addresses = addrs | ||
| } | ||
| if !changed { | ||
| return nil | ||
| } | ||
| info.SetAPIEndpoint(endpoint) | ||
| if err := info.Write(); err != nil { | ||
| return err | ||
|
|
||
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.
Why is this not an environ tag, to match the tag in api.Info ?