Feat: ai gateway file convert#2137
Conversation
harshadixit12
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2137 +/- ##
==========================================
- Coverage 36.73% 36.32% -0.41%
==========================================
Files 78 79 +1
Lines 7429 7504 +75
==========================================
- Hits 2729 2726 -3
- Misses 4455 4532 +77
- Partials 245 246 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| // Print warnings to stderr | ||
| if len(warnings) > 0 { | ||
| for _, warning := range warnings { | ||
| fmt.Fprintf(os.Stderr, "Warning: %v\n", warning) |
There was a problem hiding this comment.
This would break json output, right?
Are we concerned about that right now?
There was a problem hiding this comment.
I see that we are using YAML everywhere for now. We can revisit this later. Let's add a TODO comment on top so that we don't miss this when we add JSON support.
There was a problem hiding this comment.
Ideally - yes, we should - but i'll get to this once all PRs have been merged.
| return fmt.Errorf("failed to read source file: %w", err) | ||
| } | ||
|
|
||
| converted, warnings, err := convert.Convert(sourceContent, convert.Options{ |
There was a problem hiding this comment.
nitpick: Can we import this as ai2kong and call ai2kong.Convert()?
It will be more verbose and avoid the confusing repetition.
| } | ||
|
|
||
| func validateAi2KongFlags(_ *cobra.Command, _ []string) error { | ||
| if convertSourceFile == "" { |
There was a problem hiding this comment.
We can take only one file at a time for conversion, right?
If yes, can we add a validation for that too.
There was a problem hiding this comment.
For flag type of string - cobra follows "last value" wins - so additional validation is not needed. (same in openapi2kong)
| if docMap, ok := doc.(map[string]interface{}); ok { | ||
| if infoMap, ok := docMap["_info"].(map[string]interface{}); ok { | ||
| // _info exists, update select_tags | ||
| infoMap["select_tags"] = []string{"managed-by:deck-ai"} |
There was a problem hiding this comment.
This can over-write the existing select-tags.
There was a problem hiding this comment.
This is converting an ai gateway file -> deck, the ai gateway file won't have select tags at global level
| } else { | ||
| // _info doesn't exist, create it with select_tags | ||
| docMap["_info"] = map[string]interface{}{ | ||
| "select_tags": []string{"managed-by:deck-ai"}, |
There was a problem hiding this comment.
managed-by:deck-aishould be a constant- Elsewhere, the convention I have seen is
managed_by. Should we follow that or is this one a request from product?
There was a problem hiding this comment.
Will be made constant - this was a suggestion, not a request. I'll update it to managed_by.
| cmd := &cobra.Command{ | ||
| Use: "ai2kong", | ||
| Short: "Generate Kong configuration from AI Gateway configuration", | ||
| Long: `This command takes an AI Gateway 2.0 entity model and converts it to a standard decK state file`, |
There was a problem hiding this comment.
Since we are adding a tag ourselves for our internal working, I think we should explicitly call that out in the description. It will help endusers to know that the tag addition is expected and they shouldn't remove it.
| } | ||
|
|
||
| // Parse the converted YAML to add select_tags to _info | ||
| var doc interface{} |
There was a problem hiding this comment.
nit: I feel L64 to L81 can be abstracted out in a different function.
| defer outFile.Close() | ||
| outputWriter = outFile | ||
| } else { | ||
| outputWriter = os.Stdout |
There was a problem hiding this comment.
This can be the default IMO rather than adding a nesting here with else.
| } | ||
|
|
||
| // marshalToYAML encodes v as YAML using a two-space indent. | ||
| func marshalToYAML(v interface{}) ([]byte, error) { |
There was a problem hiding this comment.
Any reason for a separate func for this?
| err = yaml.Unmarshal([]byte(output), ¤tOutput) | ||
| require.NoError(t, err) | ||
|
|
||
| assert.Equal(t, expectedOutput, currentOutput) |
There was a problem hiding this comment.
Should we consider syncing the output to an ai gateway for validity?
We need to support the decomposed flow as well.
There was a problem hiding this comment.
I was planning to do that in the tests for sync and dump in the chained PR
3 participants
Summary
In this PR, we are adding support for a new command
deck file ai2kongwhich accepts a AI gateway declarative config, and converts it to kong 3.x format declarative config to be used with on prem AI Gateway.Implements
#2148