feat(sync/diff) Create workspace if needed #187
Conversation
cmd/common.go
Outdated
| // Using ListAll() to get all workspaces instead of Get() to get just | ||
| // the one to make sure there is no underlying error - Get() does not | ||
| // expose the response. | ||
| workspaces, err := rootClient.Workspaces.ListAll(nil) |
There was a problem hiding this comment.
the one to make sure there is no underlying error
Yes, this is a limitation and go-kong needs to improve to fix this problem but I still think we should use Get() instead. You can use IsNotFoundErr() to check if it was a 404 or not.
If there is an underlying error, return it.
There was a problem hiding this comment.
Makes sense, and done.
|
This is great work and provides a very good end-user experience but I feel this is more complicated than it should be.
Also, please also take a look at #186 which will need to work with this. |
Sure, so that we be something close to the first commit (which already worked for The reason I expanded to the full solver/syncer is that without it the workspace doesn't appear in the entity counter at the end. But if that's not a concern I agree there is a much simpler way to do this and I'll just go back to the first commit and expand it. Let me know.
I agree workspace management is too far out for decK at this point. But - this PR alone does not commit us in any way to go in this direction - there never has to be a further change to this code.
Good shout, I'll test that scenario. What's probably required is to be able to disable workspace checking and creating completely with that flag - but need to take a closer look. |
Pretty much. The only thing to note is that
That's could be considered a little odd but if we are taking the stance that workspace management is not something deck is responsible for, that's okay in my opinion.
This PR doesn't intend to do that but in the current for, it has the potential to give new contributors ideas around what could be done with workspaces and decK. OSS users can and do sometimes get very creative, which is not always good from a Software Engineering standpoint. Ping me once you have reworked this one. Thanks! |
|
@hbagdi Alright, I've simplified the logic. The output is now: $ deck sync -s workspace-demo.yaml -s openid-connect-okta.yaml
creating workspace Demo
creating service mockbin-okta
creating route okta
creating plugin openid-connect for route ff9dcb66-0aae-45ef-abad-c627ebd2201c
Summary:
Created: 3
Updated: 0
Deleted: 0The created workspace no longer appears as a created entity in the summary - but the code is a lot simpler. |
With this patch, decK creates a workspace in Kong Enterprise during if one doesn't exist. The follow behavior is intentional: - `deck diff` doesn't create a workspace; this is to preserve read only nature of the diff command - `deck sync` creates a workspace if one is not present; it should be noted that the behavior of creating a workspace via API vs via UI (Kong Manager) is different. Kong Manager creates default RBAC roles for the workspace. - Workspace meta data cannot be adjusted using decK. - `deck reset` doesn't delete a workspace. Deleting a workspace is, unfortunately, a pretty complicated operation in Kong Enterprise requiring emptying a workspace, an operation decK is not capable of doing at this point of time. In summary, decK only creates a workspace and doesn't treat workspace as a regular Kong entity. From #187
With this patch, decK creates a workspace in Kong Enterprise during if one doesn't exist. The follow behavior is intentional: - `deck diff` doesn't create a workspace; this is to preserve read only nature of the diff command - `deck sync` creates a workspace if one is not present; it should be noted that the behavior of creating a workspace via API vs via UI (Kong Manager) is different. Kong Manager creates default RBAC roles for the workspace. - Workspace meta data cannot be adjusted using decK. - `deck reset` doesn't delete a workspace. Deleting a workspace is, unfortunately, a pretty complicated operation in Kong Enterprise requiring emptying a workspace, an operation decK is not capable of doing at this point of time. In summary, decK only creates a workspace and doesn't treat workspace as a regular Kong entity. From #187
Adds the workspace as another entity to decK.
Some notes on the implementation:
_workspaceproperty. Deleting a workspace is quite tricky esp. when the Dev Portal is enabled.syncanddiff- other workspace-related behavior e.g. inresetanddumpremains unchanged (though some code has changed).masterofgo-kongfor workspace support.go-kongdoes not offer complete support for workspaces a separaterootClientis needed to create a workspace. Fortunately, the code changes for that were not too bad.file/schema.goare not from this PR - somewhere something changed and it wasn't updated.Example output:
Resolves #158.