Skip to content

Conversation

@msavy
Copy link
Member

@msavy msavy commented Aug 29, 2017

Continuation of: #27

New Features

  • Allow Apiman Gateway API to be driven directly.
  • New Apiman Gateway API list entity functions (e.g. list orgs, APIs, Clients, Versions, etc).
  • Directly apply YAML declarations on the Gateway API (with status checks, etc).
  • Generate headless Gateway JSON config from the YAML declaration format.

Rationale

  • Rejig the order of commands so that the root is now:

    • apiman gateway ...
    • apiman manager ...
  • Shares the same fundamental data model as manager; with a small tweak to policy which is non-breaking for the manager functionality.

  • Allows some simple commands to be run directly (rather than by declaration) such as retire, unregister, etc.

  • For both inbuilt and plugin policies it resolves the correct information needed to successfully interact with the Gateway API. This should substantially simplify things.

    • Plugins are resolved using the Manager libraries, so it should function near identically.
# Simple apiman example
---
  system:
    gateways:
      - name: "test-gw"
        type: "REST"
        config:
          endpoint: "http://localhost:8080/apiman-gateway-api"
          username: "apimanager"
          password: "apiman123!"
    plugins:
      - name: TestPolicy
        groupId: "io.apiman.plugins"
        artifactId: "apiman-plugins-test-policy"
        version: "1.3.1.Final"
  org:
    name: "test"
    apis:
      - name: "example"
        version: "1.0"
        config:
          endpoint: "http://localhost:8080/services/echo"
          endpointType: "rest"
          public: true
          gateway: "test-gw"
        policies:
          - name: "CachingPolicy"
            config:
              ttl: 60
          - plugin: TestPolicy
            config:
              foo: 123
$ apiman-cli gateway apply -f /Users/msavy/tmp/sample.yaml

Switched to JCommander

Switch to JCommander to allow more flexible architecture.

  • Descriptions for each subcommand.
  • Custom Git-style #usage/#help to avoid information overload.
  • Altered initialisation to allow JCommander to use utilise subcommand and help functionality more extensively.
  • Same Guice injector for all instances.
  • Much easier to do composition vs args4j tending to prefer inheritance which became an issue when dealing with things.

Here's the new help layout. All thoughts welcome...

[msavy@mmbp apiman-cli](attempt3-3-rb-2)$ ./apiman manager --help
Usage: manager [--debug] [--help, -h] <command> [<args>]

The following commands are available:

   plugin: Manage plugins
   org: Manage organisations
   apply: Apply Apiman Manager declaration
   api: Manage APIs
   gateway: Manage gateways

The following arguments are available:

   --debug: Log at DEBUG level
   --help, -h: Display usage only

And another:

[msavy@mmbp apiman-cli](attempt3-3-rb-2)$ ./apiman manager api list --help
Usage: list --orgName, -o [--debug] [--help, -h] [--server, -s] [--serverPassword, -sp] [--serverUsername, -su] [--serverVersion, -sv]
The following arguments are available:

   --orgName, -o: Organisation name
   --debug: Log at DEBUG level
   --help, -h: Display usage only
   --server, -s: Management API server address [default: http://localhost:8080/apiman]
   --serverPassword, -sp: Management API server password [default: admin123!]
   --serverUsername, -su: Management API server username [default: admin]
   --serverVersion, -sv: Management API server version [default: v12x]

Issues

  • Will almost certainly want to do some package refactoring and renaming. Would like to discuss that with community.

  • Might want to be cleverer about sorting of certain parameters, and more clearly indicate mandatory ones.

TODO

  • Unregister clients and retire APIs
  • Direct gateway listing of entities (such as orgs, APIs, clients, etc)
  • Tests
  • Client functionality (see also work by @jquantin)
  • Generate headless gateway JSON configs from YAML declaration format.

@msavy
Copy link
Member Author

msavy commented Aug 31, 2017

Here's the gateway format documented informally:

# Simple apiman example
---
  system:
    gateways:
      - name: "test-gw"
        type: "REST"
        config:
          endpoint: "http://localhost:8080/apiman-gateway-api"
          username: "apimanager"
          password: "apiman123!"
    plugins:
      - name: test-plugin # friendly name
        groupId: "io.apiman.plugins"
        artifactId: "apiman-plugins-test-policy"
        version: "1.2.4.Final"
      - name: another-test-plugin
        groupId: "io.apiman.plugins"
        artifactId: "apiman-plugins-test-policy-2"
        version: "1.2.7.Final"

  org:
    name: "test"
    apis:
      - name: "example"
        version: "1.0"
        config:
          endpoint: "http://localhost:8080/services/echo"
          endpointType: "rest"
          public: true
          gateway: "test-gw"
        policies:
          - name: CachingPolicy
            config:
              ttl: 60
          - plugin: test-plugin
            config:
               Foo: 17
               Bar: "yes"
          - name: PolicyTwo # Can be elided if only one policyDef in plugin.
            plugin: another-test-plugin
            config:
               Foo: 17
               Bar: "yes"

@msavy
Copy link
Member Author

msavy commented Nov 4, 2017

It's now possible (with some changes on the apiman side also) to:

  • List entities (orgs, clients, APIs, etc)
  • Generate JSON headless configurations from declaration!
    • This can be to STDOUT or file(s).
    • A declaration can correspond to multiple configs, this works!
      • If you just provide a directory output instead of filename(s) then one will be generated from the gateway name
      • If no usable gateway name was provided (e.g. only consisting of weird whitespace characters), it'll just be indexed as unnamed-config-N.

I need to refactor to finally fix the awful package structuring/naming I've still not solved. Will try to do that soon as I have some ideas.

Example:

./apiman-cli gateway generate headless -f /Users/msavy/tmp/sample.yaml -o /tmp/foo.json

[msavy@MacBook-Pro apiman-cli](attempt3-3-rb-2)$ cat /tmp/foo.json

{
  "apis" : [ {
    "publicAPI" : true,
    "organizationId" : "test",
    "apiId" : "example",
    "version" : "1.0",
    "endpoint" : "http://localhost:8080/services/echo",
    "endpointType" : "rest",
    "endpointContentType" : null,
    "endpointProperties" : { },
    "parsePayload" : false,
    "apiPolicies" : [ {
      "policyJsonConfig" : "{\n  \"ttl\" : 60\n}",
      "policyImpl" : "class:io.apiman.gateway.engine.policies.CachingPolicy"
    }, {
      "policyJsonConfig" : "{\n  \"foo\" : 123\n}",
      "policyImpl" : "plugin:io.apiman.plugins:apiman-plugins-test-policy:1.3.1.Final:war/io.apiman.plugins.test_policy.TestPolicy"
    } ],
    "maxPayloadBufferSize" : 0
  } ],
  "clients" : [ ]

Here's the definition:

# Simple apiman example
---
  system:
    gateways:
      - name: "test-gw"
    plugins:
      - name: TestPolicy
        groupId: "io.apiman.plugins"
        artifactId: "apiman-plugins-test-policy"
        version: "1.3.1.Final"
  org:
    name: "test"
    apis:
      - name: "example"
        version: "1.0"
        config:
          endpoint: "http://localhost:8080/services/echo"
          endpointType: "rest"
          public: true
          gateway: "test-gw"
        policies:
          - name: "CachingPolicy"
            config:
              ttl: 60
          - plugin: TestPolicy
            config:
              foo: 123

@msavy
Copy link
Member Author

msavy commented Nov 4, 2017

BTW guys I'd hugely appreciate your feedback. I can't hold off much longer and am going to have to do a release as people are dying for the various new stuff, bug-fixes, etc!

@msavy msavy self-assigned this Nov 5, 2017
@outofcoffee
Copy link
Collaborator

Hi Mark, thanks for implementing this. It looks like a really useful feature for the headless gateway feature.

I'll have a deeper look this week. Did you have any thoughts about the package naming?

@msavy
Copy link
Member Author

msavy commented Nov 6, 2017

One of the problems is that there is so much repetition of terminology that it can get a touch confusing at times! In particular gateway has meaning on the manager-api and gateway-api side, but subtly different.

So, I've been thinking of structuring it (vaguely) as:

io.apiman.cli.gateway-api.[gateway-only-things]
io.apiman.cli.manager-api.[manager-only-things]
io.apiman.cli.model.* etc

There's a bit more cross-pollination than you might think, but generally that seems an okay start.

At least then when you get something along the lines of io.apiman.cli.manager-api.gateway it's clearer what that means

WDYT?

@msavy
Copy link
Member Author

msavy commented Nov 6, 2017

Okay, naming/structure changes made. I'm still 100% open, so hit me with your feedback.

@msavy
Copy link
Member Author

msavy commented Nov 7, 2017

This passes locally but because there isn't a docker image for 1.3.2-SNAPSHOT it's breaking at the moment. Will look at how I can solve that.

@msavy
Copy link
Member Author

msavy commented Nov 9, 2017

I've added an annotation whereby a command can be labelled as being available since a particular version. A version check is performed on the remote gateway API to determine whether it's >= to the indicated version.

This helps avoid a bunch of confusing errors where version incompatibilities exist:

@CommandAvailableSince("1.3.2")

See ListOrgCommand for an example.

@msavy
Copy link
Member Author

msavy commented Nov 13, 2017

This is ready to merge from my end -- it's getting so big now that I'd prefer to do other work on a fresh PR(s).

@outofcoffee are you happy for me to merge in and then I'll rework stuff based upon any suggestions or directions you have (when you have time)?

Or perhaps I should do the release from my own fork on this occasion and then we can figure out merging into here later?

Happy to do what you prefer.

@msavy
Copy link
Member Author

msavy commented Nov 14, 2017

Green 🎉

@msavy
Copy link
Member Author

msavy commented Nov 20, 2017

Really sorry guys, I can't wait any longer.

I'm going to continue working off on my own branch and do a release from there.

@msavy
Copy link
Member Author

msavy commented Nov 20, 2017

Hey, @jcechace and @mijaros if you guys also have time to look at this PR and play with the code that'd also be awesome-o :-)!

@msavy
Copy link
Member Author

msavy commented Nov 20, 2017

I didn't notice there was a bunch of work on @outofcoffee's develop branch 😭. We should work to bring any changes there back into the codebase when you're ready.

@outofcoffee
Copy link
Collaborator

Hi @msavy - thanks again for working on this.

I’ve been testing this internally to get some feedback.

Main comments so far:

  • it’d be good to rebase this onto the develop branch - this has a number of improvements to the code structure as well as factoring out the declarative logic into a set of separate services (also making it more testable)
  • we should probably match the Docker image version in the Travis config to the one in the Gradle build script to ensure they’re in alignment
  • wherever we have locals declared we should prefer immutability, so declaring them final to keep with the existing convention
  • new package names look good
  • new commands look good
  • if we’re using JCommander let’s kill the args4j references in the build script

@msavy
Copy link
Member Author

msavy commented Nov 20, 2017

Thanks for the feedback!

it’d be good to rebase this onto the develop branch - this has a number of improvements to the code structure as well as factoring out the declarative logic into a set of separate services (also making it more testable)

Yes, sadly I didn't spot that when I started the work. I'll do it tomorrow.

Also: I'm trying to work on the client and plan changes contributed by other community members, but I have some concerns. Perhaps we can discuss that later. I might be able to just fix it up myself; we'll see by tomorrow evening!

we should probably match the Docker image version in the Travis config to the one in the Gradle build script to ensure they’re in alignment

Agreed; will do. That was just an oversight.

wherever we have locals declared we should prefer immutability, so declaring them final to keep with the existing convention

Okay, sounds good.

new package names look good
new commands look good
if we’re using JCommander let’s kill the args4j references in the build script

Will do; was an oversight.

@msavy msavy changed the base branch from master to develop November 28, 2017 12:04
@msavy
Copy link
Member Author

msavy commented Nov 28, 2017

@outofcoffee Please let me know if this gets the thumbs up. The work to rebase is all done and tested.

Should be okay to merge now.

I'll do docs as separate PR as this is already too large, I think!

outofcoffee and others added 3 commits November 29, 2017 11:35
… run commands. Reorganises command package structure. Factors out declarative application logic into separate services. Improves docs.
- Rejig the order of commands so that the root is now:
  - apiman gateway ...
  - apiman manager ...

Shares the same fundamental data model as manager; with a small tweak to policy which is non-breaking for the manager functionality.

For both inbuilt and plugin policies it resolves the correct information needed to successfully interact with the Gateway API. This should substantially simplify things.

Plugins are resolved using the Manager libraries, so it should function near identically.

Switch to JCommander to allow more flexible architecture.

- Descriptions for each subcommand.

- Custom Git-style #usage/#help to avoid information overload.

- Altered initialisation to allow JCommander to use utilise
  subcommand and help functionality more extensively.

- Same Guice injector for all instances.

- Add retire and unregister commands for API and Client, respectively

- Add brackets around mandatory parameters

- Add commands for using new API that allows listing of entities.

- Allow generation of headless gateway config from YAML declaration

- Refactor project to separate out 'gatewayapi' and 'managerapi' functionality into own packages.

- Add version checks on a per-command basis (see @AvailableSince annotation).

- Add gateway status command. Prints Gateway API status JSON.

- Use master build of Apiman to access latest API changes and test.

- Tests for new functionality
@msavy msavy changed the base branch from develop to master November 29, 2017 12:17
Copy link
Collaborator

@outofcoffee outofcoffee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased onto develop as discussed. Just awaiting CI to go green, then will merge.

@outofcoffee outofcoffee changed the base branch from master to develop November 29, 2017 13:33
@outofcoffee outofcoffee merged commit c07f4f2 into apiman:develop Nov 29, 2017
@msavy
Copy link
Member Author

msavy commented Nov 29, 2017

Woo, thanks!

@outofcoffee
Copy link
Collaborator

Awesome work, thanks @msavy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants