Skip to content

Support for 0.14.0 - services, routes, and other compatibility changes.#150

Open
dgradl wants to merge 2 commits into
mybuilder:masterfrom
dgradl:Kong0140
Open

Support for 0.14.0 - services, routes, and other compatibility changes.#150
dgradl wants to merge 2 commits into
mybuilder:masterfrom
dgradl:Kong0140

Conversation

@dgradl
Copy link
Copy Markdown

@dgradl dgradl commented Sep 24, 2018

This provides support for Kong 0.13.0 Services and Routes. Specifically it was tested against 0.14.0. I tried to follow the patterns throughout the existing codebase. Unit and integration tests have been added and all tests are passing. I tried to allow for backwards compatibility, but there were some breaking API changes in 0.14.0 with certificates and I could not think of a way with the routes to deal with the compatibility change. Also, though I Initially replicated from API Plugins I removed the consumer based plugins thinking it wasn't supported but it is. I don't really have a use case for it so I don't have time immediately to go back and update that.

#132

@dgradl dgradl force-pushed the Kong0140 branch 2 times, most recently from a2a4fbf to 9420f67 Compare September 24, 2018 19:36
@dgradl
Copy link
Copy Markdown
Author

dgradl commented Sep 25, 2018

Ok I don't know why snyk is complaining - I've run it locally and it passes. Clicking details doesn't help because I do not have permissions.

@gavinlove
Copy link
Copy Markdown
Member

@dgarlitt snyk is a security tool it is scanning for potentially vulnerable packes konfig depends on. Here is the details although this is not specific to your branch.

screencapture-app-snyk-io-org-gavinlove-test-github-7d57c134-8f76-4551-a4d0-8c21aba82091-9420f67ac2c7457f0be6245079608dfda77cca8a-2018-09-25-13_26_36

@dgradl
Copy link
Copy Markdown
Author

dgradl commented Sep 25, 2018

Yes I understand, I installed snyk and ran a snyk test locally and it does not find the vulnerabilities you've listed there, so that is what I find strange.

@cilindrox
Copy link
Copy Markdown

cilindrox commented Sep 28, 2018

@dgradl you can try removing the existing modules and lockfile rm -r node_modules yarn.lock and then reinstalling. That should update the deps that are reporting the vulns.
Also, you can test locally via npm audit and that should provide you some quick fixes.

Edit: keep in mind you'll need to add the updated lockfile to the pr

@mynameisgv
Copy link
Copy Markdown

@dgradl I was looking at your branch Kong0140. When adding a new Route i see a PUT being made instead of POST. Kong document also says it is a POST. (https://docs.konghq.com/0.14.x/admin-api/#add-route)
Can you please check on this.

@dgradl
Copy link
Copy Markdown
Author

dgradl commented Sep 29, 2018

It is undocumented. Routes have no name so it's difficult to support update or remove in kongfig without some reference.
I used a little bit of code that will convert a string "name" into a uuid and the put allows you to create routes with a given uuid.

Kong/kong#3416

@thibaultcha
Copy link
Copy Markdown

FWIW, Routes are getting a name in 1.0 :)

If you see undocumented methods, we will backlog it and fix it on our side, but would also welcome a PR to our documentation!

Great job @dgradl!

@mynameisgv
Copy link
Copy Markdown

@dgradl I tried using the Branch for a simple service. I get
PUT http://localhost:8001/routes/7c92cf1e-ee8d-39cc-85f8-355a3d6e4b86
{ hosts: [ 'foo.com' ],
paths: [],
methods: [],
service: { id: 'db7be8c1-7555-4349-aca4-292c6a659902' } }
405 Not Allowed { message: 'Method not allowed' }
Error: Not Allowed
{"message":"Method not allowed"}

Error: Not Allowed
{"message":"Method not allowed"}

at /opt/Apps/Stash/kongfig/lib/core.js:394:45
at process._tickCallback (internal/process/next_tick.js:68:7)

Please take a look.

@dgradl
Copy link
Copy Markdown
Author

dgradl commented Sep 30, 2018

Check your kong version. The integration tests validate that it works. I've also already started using this branch and migrated 50 of my apis to service/routes.

@mynameisgv
Copy link
Copy Markdown

mynameisgv commented Sep 30, 2018

@dgradl I use 0.33 enterprise edition.
Even the integration test produce the same result.

FAIL test-integration/service.test.js (10.718s)
● Service routes › should add mockbin service with a route

Not Allowed
{"message":"Method not allowed"}

  392 |
  393 |                             if (!response.ok) {
> 394 |                                 var error = new Error(response.statusText + '\n' + content);
      |                                             ^
  395 |                                 error.response = response;
  396 |
  397 |                                 throw error;

  at lib/core.js:394:45

@dgradl
Copy link
Copy Markdown
Author

dgradl commented Sep 30, 2018

Its tested against 0.14 community. As the PR indicates. I don't know what version of enterprise that Kong PR I referenced would be merged into. I certainly have no way to test against enterprise edition and I make no claim that this works with it.

@mynameisgv
Copy link
Copy Markdown

mynameisgv commented Oct 1, 2018

Hello @dgradl
Thanks for your response.
Actually i was wrong on the PUT support.
Kong does not support a PUT (update/create route). It only does POST, PATCH , GET and Delete.
0.33 is same as 0.13.x . Even 0.13 does not support PUT.
only 0.14 does.

I made some changes and got it working from 0.13. However since there is no way to find if a route already exists under a service, it keeps creating duplicate routes. For example if run
the below definition 2 times it creates 2 routes under the service but only 1 services.
The first run makes a POST CALL to create a service and the second one goes as PATCH and avoids duplicating. However when creating the route it always goes as POST and ends up duplicating.
@thibaultcha Any way to overcome this problem. (0.13 and 0.33 ee)

{
"services": [{
"name": "mockbin",
"ensure": "present",
"attributes": {
"url": "http://mockbin.com/test"
},
"routes": [{
"name": "r1",
"attributes": {
"hosts": ["foo.com"],
"paths": [],
"methods": []
}
}]
}]
}

@dgradl
Copy link
Copy Markdown
Author

dgradl commented Oct 1, 2018

Yes I gave up on 0.13 support for that reason. Without a name or the put how can you associate the route in your yaml with the route in kong. You would have to match on hosts, paths, and methods. Except that any update you make would look like a new route. You could just delete all routes and replace every time. But this would result in your api being temporarily unavailable each time you run kongfig.

@rodriamaro
Copy link
Copy Markdown

any new about supporting routes and services?

@lightsaway
Copy link
Copy Markdown

lightsaway commented Nov 21, 2018

any news?

@cilindrox
Copy link
Copy Markdown

We've been using this to automate k8s and Docker deployments: https://github.com/liyuntao/kong-init
Needs a bit more spit and polish, but the maintainer has been awesome handling contributions.

@mrgavinconway001
Copy link
Copy Markdown

Anything on this? I'd really like to be able to use this.

@dgradl
Copy link
Copy Markdown
Author

dgradl commented Jan 11, 2019 via email

@gudzenkov
Copy link
Copy Markdown

gudzenkov commented Jan 22, 2019

I really struggled to get snyk to be happy with the dependencies as there were vulnerabilities before I even made updates. But I will take another stab at it soon.

Hi Dan,
We are looking into Kong upgrade to 0.33 EE utilizing new data model but we stuck on kongfig not yet supporting it.
Do you have an ETA on dependencies issue?

@dgradl
Copy link
Copy Markdown
Author

dgradl commented Mar 30, 2019

Finally snyk is happy. Please approve.

@mrgavinconway001
Copy link
Copy Markdown

@dgradl does your PR fix service and route backups?

@mynameisgv
Copy link
Copy Markdown

@dgradl I tested this branch and Looks like this version does not handle pagination.
#158

@dgradl
Copy link
Copy Markdown
Author

dgradl commented Apr 3, 2019

pagination? service and route backups? Not really sure what you all mean. But like I don't really think anyone will ever merge this and I don't really care anymore. I've been using it to manage my Kong environment for the past 6 months.

@eraye1
Copy link
Copy Markdown

eraye1 commented May 21, 2019

hey all, can we please get this merged in?

@rendyfebry
Copy link
Copy Markdown

This one need to be merged

@mrgavinconway001
Copy link
Copy Markdown

Anyone fancy clicking that magic merge button?

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.