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

kong2kic changes after code review #1218

Merged
merged 1 commit into from
Feb 15, 2024
Merged

kong2kic changes after code review #1218

merged 1 commit into from
Feb 15, 2024

Conversation

battlebyte
Copy link
Collaborator

@battlebyte battlebyte commented Feb 13, 2024

This change incorporates the suggestions from
@Gabriele Gerbino

@travenichka

@michal

@Greg

  • Use of KongUpstreamPolicy in 3.x
  • Use labels for credential type in 3.x
  • No status or metadata.creationTimestamp fields are in the output

The new command interface is:

Convert Kong configuration files to Kong Ingress Controller (KIC) manifests.

The kong2kic subcommand transforms Kong’s configuration files, written in the deck format,
into Kubernetes manifests suitable for the Kong Ingress Controller. By default kong2kic generates
manifests for KIC v3.x using the Kubernetes Gateway API. Only HTTP/HTTPS routes are supported.

Usage:
  deck file kong2kic [flags]

Flags:
      --class-name string    Value to use for "kubernetes.io/ingress.class" ObjectMeta.Annotations and for
                             		"parentRefs.name" in the case of HTTPRoute. (default "kong")
  -f, --format string        output file format: json or yaml. (default "yaml")
  -h, --help                 help for kong2kic
      --ingress              Use Kubernetes Ingress API manifests instead of Gateway API manifests.
      --kicv2                Generate manifests compatible with KIC v2.x.
  -o, --output-file string   Output file to write. Use - to write to stdout. (default "-")
  -s, --state string         decK file to process. Use - to read from stdin. (default "-")

@battlebyte battlebyte requested a review from a team February 13, 2024 10:56
Copy link
Member

@mheap mheap left a comment

Choose a reason for hiding this comment

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

Merging in to the kic branch for final review

@mheap
Copy link
Member

mheap commented Feb 13, 2024

Aha! I'm not on @Kong/team-deck. Could someone please merge? This is targeting the kic branch, not main and we'll do a final review on #1050

@Tieske
Copy link
Member

Tieske commented Feb 13, 2024

looking at the commits, it should be rebased on the kic branch first.

@battlebyte
Copy link
Collaborator Author

My battlebyt/kic fork is already in sync with Kong:kic branch, correct?

@czeslavo
Copy link
Contributor

My suggestion regarding dropping status and creationTimestamp fields has been addressed. 👍 I'm not sure about failing tests though - as I can see they're passing on #1050 so probably there's something to be fixed here before merging to kic branch.

@CLAassistant
Copy link

CLAassistant commented Feb 13, 2024

CLA assistant check
All committers have signed the CLA.

@Tieske
Copy link
Member

Tieske commented Feb 14, 2024

My battlebyt/kic fork is already in sync with Kong:kic branch, correct?

Doesn't look like it. If you look at the commits in this PR, they have a lot of work that is also in the base branch.

Remove status and timestamp

Linter and simplify

Remove unused builders

Simplify output generation

deps update
@battlebyte
Copy link
Collaborator Author

Sorry @Tieske , you're right. This pull request now has 1 squashed commit that continues after the last commit (f419701) in PR #1050.

@Tieske Tieske merged commit 9e8a5bb into Kong:kic Feb 15, 2024
1 of 37 checks passed
@Tieske
Copy link
Member

Tieske commented Feb 15, 2024

@battlebyte I fixed the linter issues, and then accidentally merged this PR because its name is the same as the base branch.

Please ensure you change the branch name next time. (if you have no other pending work, my suggestion would be to delete your clone and create a new one all together)

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.

5 participants