Add `cb role` command and other updates. #18

abrightwell · 2022-03-23T16:29:39Z

This PR adds the cb role command to allow for managing cluster roles. As well, it updates the cb uri command to allow for a role name to be provided via the --role flag.

> ./bin/cb role --help
Usage: cb role <create|update|destroy>
    -h, --help                       Show this help
    create                           Create new role for a cluster
    update                           Update a cluster role
    destroy                          Destroy a cluster role

> ./bin/cb role create --help
Usage: cb role create <--cluster>
    -h, --help                       Show this help
    --cluster ID                     Choose cluster

> ./bin/cb role update --help
Usage: cb role update <--cluster> <--name> [--mode] [--rotate-password]
    -h, --help                       Show this help
    --cluster ID                     Choose cluster
    --name NAME                      Role name
    --read-only <true|false>         Read-only
    --rotate-password <true|false>   Rotate password
    
> ./bin/cb role destroy --help
Usage: cb role destroy <--cluster> <--name>
    -h, --help                       Show this help
    --cluster ID                     Choose cluster
    --name NAME                      Role name

> ./bin/cb uri --help
Usage: cb uri <cluster id> [--role]
    -h, --help                       Show this help
    --role NAME                      Role name (default: default)

Removes unused function from `scope.cr`.

Previously we refactored the use of `print_team_slash_cluster` to be a method defined on `Action`. When refactoring uses of this method, we missed one the `psql` action. These changes simply clean this up and brings the action in line with the rest of the source.

Adds some convenience methods to the Client::Error class for `403 Forbidden` and `400 Bad Request`.

Adds cluster role methods to `CB::Client`. As well, updates calls to get the default role to use new method that allows specifying the name of the cluster role to retrieve. The URI on the `role` record was updated to be nilable, as it is possible for the API to return a null value for URI when a role is deleted.

spec/cb/cluster_uri_spec.cr

abrightwell · 2022-03-23T16:32:43Z

spec/cb/cluster_uri_spec.cr

+    c.p_get_role = -> : CB::Client::Role {
+      raise CB::Client::Error.new("", "",
+        HTTP::Client::Response.new(HTTP::Status::BAD_REQUEST))
+    }


I'm sure this could be 'simpler' too...

Yeah we could take a look at patterns in our usage of them in the test and make more helper methods. Idk if we have enough now to figure out what'd be useful, or if it'd make sense to wait for a few more to accumulate

abrightwell · 2022-03-23T16:35:29Z

src/cb/action.cr

+    # Nilable boolean setter. This is useful for fields where nil can have a
+    # meaningful value beyond being falesy.
+    macro bool_setter?(property)


I needed this so that we could definitively know when a particular flag was set and not simply omitted. For example, --read-only on cb role update. If we just defaulted to false then it would update the role every time, possibly setting the 'mode' or 'flavor' of the role to be be write when it wasn't intended. 😬

I think we define one of these in terms of the other?, or have one that takes an option. But it's fiddly stuff and I can take a look after it's merged.

src/cb/client.cr

src/cb/cluster_uri.cr

src/cb/psql.cr

src/cb/role.cr

src/stdlib_ext.cr

brandur · 2022-03-23T18:16:16Z

src/cb/client.cr

+  end
+
+  # https://crunchybridgeapi.docs.apiary.io/#reference/0/clustersclusteridrolesrolename/update-role
+  def update_role(cluster_id, role_name, ur)


OOC, what's "ur"? Might be better to call this a body or request body if that's what it is.

heh... yeah... ur -> updated role... I was just following other similar methods for consistency. But yeah, I definitely agree it could be more clear on what it is.

brandur

Seems good from my perspective!

One question. These usage docs that are generated:

> ./bin/cb uri --help
Usage: cb uri <cluster id> [--role]
    -h, --help                       Show this help
    --role NAME                      Role name (default: default)

Would it be possible to eventually have some sort of docstring on there describing what the command does? Some are reasonably obvious, but IMO some like this aren't — "uri" is a pretty generic term, and I'd have to guess that what "cb uri" does is give you a Postgres connection string.

abrightwell · 2022-03-23T18:32:02Z

Seems good from my perspective!

One question. These usage docs that are generated:
> ./bin/cb uri --help
Usage: cb uri <cluster id> [--role]
    -h, --help                       Show this help
    --role NAME                      Role name (default: default)
Would it be possible to eventually have some sort of docstring on there describing what the command does? Some are reasonably obvious, but IMO some like this aren't — "uri" is a pretty generic term, and I'd have to guess that what "cb uri" does is give you a Postgres connection string.

Yeah, I think there can/should be some improvements around this. I was also looking into the potential behind having some man pages we could generate for it as well to bring more details to all of it as well.

Also, another improvement I want to make is around the flag descriptions themselves. Would be great to have more detail there in some way.

will

Just the one small thing I think. I can take care of it tomorrow if you don’t get to it first.

src/cb/role.cr

src/stdlib_ext.cr

will · 2022-03-30T00:26:12Z

src/cb/action.cr

+    # Nilable boolean setter. This is useful for fields where nil can have a
+    # meaningful value beyond being falesy.
+    macro bool_setter?(property)


I think we define one of these in terms of the other?, or have one that takes an option. But it's fiddly stuff and I can take a look after it's merged.

Adds support to `create`, `update` and `destroy` a cluster role. Also, the `cb uri` command is updated to allow for specifying the name of the role to retrieve.

``` <<< /nix/store/6sk9jhxaqa01jm0wp0b4x9labcyi5a3c-cb-3.6.7.drv >>> /nix/store/jwxavs1iq4zp0bshivkz508js1s8f6g2-cb-3.6.7.drv Version changes: [C.] #1 binutils-with-gold 2.44.tar.bz2 x2 -> 2.44.tar.bz2 [C.] #2 builder.pl <none> x3 -> <none> x2 [C.] #3 cctools-binutils-darwin-wrapper 1010.6 x7 -> 1010.6 x5 [C.] #4 clang 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #5 clang-at-least 16-LLVMgold-path.patch x4 -> 16-LLVMgold-path.patch x3 [C.] #6 clang-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #7 clang-wrapper 20.1.8 x3, 21.1.2 x3 -> 21.1.2 x3 [C.] #8 compiler-rt 20.1.8, 21.1.2 -> 21.1.2 [C.] #9 compiler-rt-libc 20.1.8, 21.1.2 -> 21.1.2 [C.] #10 compiler-rt-src 20.1.8, 21.1.2 -> 21.1.2 [C.] #11 cpio 2.15 x3, 2.15.tar.bz2 x2 -> 2.15 x2, 2.15.tar.bz2 [C*] #12 crystal 1.10.1-1-darwin-universal.tar.gz, 1.16.3 -> 1.10.1-1-darwin-universal.tar.gz, 1.18.2 [C.] #13 expand-response-params <none> x5 -> <none> x4 [C.] #14 gnu-install-dirs.patch <none> x4 -> <none> x3 [C.] #15 jq 1.8.1 x2, 1.8.1.tar.gz x2 -> 1.8.1, 1.8.1.tar.gz [C.] #16 libbfd-plugin-api-header <none> x3 -> <none> x2 [C.] #17 libcxx 19.1.2+apple-sdk-15.5 x5 -> 19.1.2+apple-sdk-15.5 x4 [C.] #18 llvm 18-compatibility.patch, 20.1.8, 21.1.2 x2 -> 18-compatibility.patch, 21.1.2 x2 [C.] #19 llvm-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #20 llvm-tblgen 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #21 llvm-tblgen-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #22 macOS-SDK 11.3 x2, 14.4, 15.5 x3 -> 11.3 x2, 14.4, 15.5 x2 [C.] #23 make-binary-wrapper-hook <none> x4 -> <none> x3 [C.] #24 onig 6.9.10.tar.gz x2 -> 6.9.10.tar.gz [C.] #25 oniguruma 6.9.10 x2 -> 6.9.10 [C.] #26 pbzx 1.0.2 x3 -> 1.0.2 x2 [C.] #27 psutil 7.1.0.tar.gz x2 -> 7.1.0.tar.gz [C.] #28 python3 3.13.8 x2, 3.13.8-env x2 -> 3.13.8 x2, 3.13.8-env [C.] #29 python3.13-psutil 7.1.0 x2 -> 7.1.0 [C*] #30 source <none> x85 -> <none> x83 [C*] #31 stdenv-darwin <none> x5 -> <none> x3 Removed packages: [R.] #1 bd49bbaaafc98433a2cb4e95ce25b7a201baf5a5.patch <none> Closure size: 1239 -> 1201 (806 paths added, 844 paths removed, delta -38, disk usage -161.7KiB). ```

* Allow temp location to change if necessary To keep the specs completely isolated, we don't want to reuse the same global tempdir. * update crystal 1.16 -> 1.18 ``` <<< /nix/store/6sk9jhxaqa01jm0wp0b4x9labcyi5a3c-cb-3.6.7.drv >>> /nix/store/jwxavs1iq4zp0bshivkz508js1s8f6g2-cb-3.6.7.drv Version changes: [C.] #1 binutils-with-gold 2.44.tar.bz2 x2 -> 2.44.tar.bz2 [C.] #2 builder.pl <none> x3 -> <none> x2 [C.] #3 cctools-binutils-darwin-wrapper 1010.6 x7 -> 1010.6 x5 [C.] #4 clang 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #5 clang-at-least 16-LLVMgold-path.patch x4 -> 16-LLVMgold-path.patch x3 [C.] #6 clang-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #7 clang-wrapper 20.1.8 x3, 21.1.2 x3 -> 21.1.2 x3 [C.] #8 compiler-rt 20.1.8, 21.1.2 -> 21.1.2 [C.] #9 compiler-rt-libc 20.1.8, 21.1.2 -> 21.1.2 [C.] #10 compiler-rt-src 20.1.8, 21.1.2 -> 21.1.2 [C.] #11 cpio 2.15 x3, 2.15.tar.bz2 x2 -> 2.15 x2, 2.15.tar.bz2 [C*] #12 crystal 1.10.1-1-darwin-universal.tar.gz, 1.16.3 -> 1.10.1-1-darwin-universal.tar.gz, 1.18.2 [C.] #13 expand-response-params <none> x5 -> <none> x4 [C.] #14 gnu-install-dirs.patch <none> x4 -> <none> x3 [C.] #15 jq 1.8.1 x2, 1.8.1.tar.gz x2 -> 1.8.1, 1.8.1.tar.gz [C.] #16 libbfd-plugin-api-header <none> x3 -> <none> x2 [C.] #17 libcxx 19.1.2+apple-sdk-15.5 x5 -> 19.1.2+apple-sdk-15.5 x4 [C.] #18 llvm 18-compatibility.patch, 20.1.8, 21.1.2 x2 -> 18-compatibility.patch, 21.1.2 x2 [C.] #19 llvm-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #20 llvm-tblgen 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #21 llvm-tblgen-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #22 macOS-SDK 11.3 x2, 14.4, 15.5 x3 -> 11.3 x2, 14.4, 15.5 x2 [C.] #23 make-binary-wrapper-hook <none> x4 -> <none> x3 [C.] #24 onig 6.9.10.tar.gz x2 -> 6.9.10.tar.gz [C.] #25 oniguruma 6.9.10 x2 -> 6.9.10 [C.] #26 pbzx 1.0.2 x3 -> 1.0.2 x2 [C.] #27 psutil 7.1.0.tar.gz x2 -> 7.1.0.tar.gz [C.] #28 python3 3.13.8 x2, 3.13.8-env x2 -> 3.13.8 x2, 3.13.8-env [C.] #29 python3.13-psutil 7.1.0 x2 -> 7.1.0 [C*] #30 source <none> x85 -> <none> x83 [C*] #31 stdenv-darwin <none> x5 -> <none> x3 Removed packages: [R.] #1 bd49bbaaafc98433a2cb4e95ce25b7a201baf5a5.patch <none> Closure size: 1239 -> 1201 (806 paths added, 844 paths removed, delta -38, disk usage -161.7KiB). ``` --------- Co-authored-by: Will Leinweber <will@bitfission.com>

abrightwell added 4 commits March 14, 2022 10:30

Remove unused function.

4a8240b

Removes unused function from `scope.cr`.

Cleanup psql action.

e7734ce

Previously we refactored the use of `print_team_slash_cluster` to be a method defined on `Action`. When refactoring uses of this method, we missed one the `psql` action. These changes simply clean this up and brings the action in line with the rest of the source.

Add error check methods to Client::Error

de1dcb0

Adds some convenience methods to the Client::Error class for `403 Forbidden` and `400 Bad Request`.

abrightwell force-pushed the abrightwell/cluster-roles branch 3 times, most recently from bc82913 to c9bd3a6 Compare March 23, 2022 16:44

abrightwell commented Mar 23, 2022

View reviewed changes

abrightwell requested review from brandur and will March 23, 2022 16:51

abrightwell force-pushed the abrightwell/cluster-roles branch from c9bd3a6 to 91ad8a8 Compare March 23, 2022 17:08

brandur reviewed Mar 23, 2022

View reviewed changes

brandur approved these changes Mar 23, 2022

View reviewed changes

will approved these changes Mar 30, 2022

View reviewed changes

Add support for cb role command.

70a38ee

Adds support to `create`, `update` and `destroy` a cluster role. Also, the `cb uri` command is updated to allow for specifying the name of the role to retrieve.

abrightwell force-pushed the abrightwell/cluster-roles branch from 91ad8a8 to 70a38ee Compare March 30, 2022 12:49

abrightwell merged commit 5863205 into main Mar 30, 2022

abrightwell deleted the abrightwell/cluster-roles branch March 30, 2022 13:06

Add cb role command and other updates. #18

Add cb role command and other updates. #18

Uh oh!

Conversation

abrightwell commented Mar 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

abrightwell Mar 23, 2022

Choose a reason for hiding this comment

Uh oh!

will Mar 30, 2022

Choose a reason for hiding this comment

Uh oh!

abrightwell Mar 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

will Mar 30, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brandur Mar 23, 2022

Choose a reason for hiding this comment

Uh oh!

abrightwell Mar 23, 2022

Choose a reason for hiding this comment

Uh oh!

brandur left a comment

Choose a reason for hiding this comment

Uh oh!

abrightwell commented Mar 23, 2022

Uh oh!

will left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

will Mar 30, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add `cb role` command and other updates. #18

Add `cb role` command and other updates. #18

abrightwell commented Mar 23, 2022 •

edited

Loading

abrightwell Mar 23, 2022 •

edited

Loading