Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deck file openapi2kong should add _info to generated decK file #1022

Open
timothystone-knsl opened this issue Sep 8, 2023 · 7 comments
Open

Comments

@timothystone-knsl
Copy link

timothystone-knsl commented Sep 8, 2023

Ideally, the deck file openapi2kong -s openapi.yaml -o deck.yaml should produced the _info element in the resulting deck file. For example, given this OpenAPI Spec with x-kong-tags:

---
# This is an annotated OpenAPI spec used to demonstrate/document the behavior of
# OpenAPI to Kong conversions.

openapi: 3.0.0

info:
  description: Learn service
  version: 1.0.0
  title: Learn Service

servers:
- url: https://{host}.konghq.com/kongu/api/v1/learn
  # the path variable {host} will be replaced by its default value below.
  # NOTE: if multiple entries, then only the first one will be used to collect the
  # protocol and path. The other entries will only be used to create Target entities.
  # "servers" objects on "path" and "operation" objects will cause additional Upstream
  # and Service entities to be created.
  description: Non production servers
  variables:
    host:
      enum:
      - alpha
      - dev
      - qa
      default: alpha
- url: https://konghq.com/kongu/api/v1/learn
  description: Production server

x-kong-tags: [ tag1, tag2 ]
  # specify the tags to use for each Kong entity generated. The tags can be overridden
  # when doing the conversion. This can only be specified on document level.
...
...

deck.yaml

_format_version: "1.1"
_info:
  select_tags:
    - tag1
    - tag2
...
...

We see the OpenAPI spec as the single source of truth in our service architecture. As deck can have the tendency to clobber a database state absent the select_tags and adding the _info element would have to occur outside a CI pipeline this missing element seems like it should be addressed.

We don't see a workflow where deck files are produced outside of a CI pipeline and independently committed to a repository by Pull or Merge Request where a manual step is necessary to assure this value is set.

@timothystone-knsl timothystone-knsl changed the title deck file openapi2kong should add _info to generated decK file deck file openapi2kong should add _info to generated decK file Sep 11, 2023
@Tieske
Copy link
Member

Tieske commented Oct 16, 2023

sorry for the long delay in responding, but had to give this some thought.

Select-tags and tags are not the same thing. Tags are a Kong property, where select-tags are a decK 'mechanism'. This also means we named the --select-tags option badly 😞, it should have been --tags

I was thinking we can add a flag to the conversion, eg;

--set-select-tags [ none | file | cli ]
	What to set as select-tags in `$._info.select-tags`; 
		- "none" will set no select-tags
		- "file" will follow the top-level `x-kong-tags` directive from the OAS file
		- "cli" will set the tags as set via `--select-tags` (mandatory if this is specified)

and probably the add-tags, list-tags and remove-tags commands should also get an option to operate on the $._info.select-tags entry.

@rspurgeon thoughts?

@timothystone-knsl
Copy link
Author

I was thinking we can add a flag to the conversion, eg;

--set-select-tags [ none | file | cli ]
	What to set as select-tags in `$._info.select-tags`; 
		- "none" will set no select-tags
		- "file" will follow the top-level `x-kong-tags` directive from the OAS file
		- "cli" will set the tags as set via `--select-tags` (mandatory if this is specified)

and probably the add-tags, list-tags and remove-tags commands should also get an option to operate on the $._info.select-tags entry.

This approach aligns well to our thinking. Looking forward to seeing the solution and can provide "beta testing."

@rspurgeon
Copy link
Collaborator

Thanks for the issue submission @timothystone-knsl and response @Tieske .

Right now x-kong-tags results in the writing of the whole set of provided tags to every entity in the generated output. Does it always make sense that these should match what you would want to pass to deck ... --select-tags ? It does limit what you can do with the resulting file as it relates to further merging (deck file merge) as we don't support merging at depth beyond top level arrays. Just trying to think through that aspect.

FWIW I think CLI interface idea may be simplified if we switch off the first non-whitespace character of the provided value.

User could specify specific tags directly on the CLI which don't have to match the file

deck file openapi2kong ... --select-tags: '[ tag1, tag2 ]'

Or they could use a selector:

--select-tags: '$.x-kong-tags'

Which also might allow for more flexibility, like this one?

--select-tags: '$.tags.[*].name'

Open to objections of course.

@timothystone-knsl
Copy link
Author

timothystone-knsl commented Oct 30, 2023

@rspurgeon are you saying the following is currently supported:

User could specify specific tags directly on the CLI which don't have to match the file

deck file openapi2kong ... --select-tags: '[ tag1, tag2 ]'
Or they could use a selector:
--select-tags: '$.x-kong-tags'
Which also might allow for more flexibility, like this one?
--select-tags: '$.tags.[*].name'

or suggesting support for? If supported, these are undocumented arguments to the --select-tags (and the use of : ... have I seen that in the documentation?) and MAY be a stop gap. Need to discuss.

Recall, many organizations see the OAS document as the Document of Record. My team believes that without $._info.select-tags being written, the loss of meta-data without explicit switches leads to unexpected Kong database behaviors that require manual intervention and add friction, in the complexity, to CI/CD configurations.

@rspurgeon
Copy link
Collaborator

rspurgeon commented Nov 1, 2023

@rspurgeon are you saying the following is currently supported:
or suggesting support for? If supported,

@timothystone-knsl I'm suggesting that if we proceeded with the suggestion to inject tags into the generated deck configuration via command line, that it could be done in the way I suggest instead of having multiple flags on the CLI. My suggestion here is just pseduo code, sorry, I did not intend for it to be exactly accurate as to the CLI keys & values. Here is my more accurate suggestion.

Allows a user to specify any array of string tags on the shell:

deck file openapi2kong -s oas.yaml \
  --select-tags "[tag1,$MY_BASH_VAR]"

Allows a user to specify tags via values in the input state file with a selctor:

deck file openapi2kong -s oas.yaml \
  --select-tags '$.x-kong-tags'

The behavior can be switched off the first character in the value of the flag. We don't need two different flags. LMK what you think

@rspurgeon
Copy link
Collaborator

rspurgeon commented Nov 1, 2023

@timothystone-knsl There is a solution that you may be able to use today that is semi-equivalent to the suggested CLI addition. You could pipe together the deck file openapi2kong with a deck file patch command to add the _info block you need. It's not as elegant, but does work today.

$ deck file openapi2kong -s oas.yaml | \
   deck file patch --selector "$" --value "_info":'{"select_tags":["tag1","tag2"]}' | \
  yq ._info

I used yq to filter on the _info tag above and get this:

select_tags:
  - tag1
  - tag2

If you want to add values from the file, then yes you'd need some more 'yaml gymnastics'.

I will add an internal tracking ticket to support the idea as discussed. Thanks for the issue.

@timothystone-knsl
Copy link
Author

timothystone-knsl commented Nov 2, 2023

I like the thinking in both approaches. I'm sharing internally as well, @rspurgeon

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

No branches or pull requests

3 participants