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

fix: changed dql routes format to show dict for service #187

Merged
merged 6 commits into from
Jan 31, 2025

Conversation

Prashansa-K
Copy link
Collaborator

@Prashansa-K Prashansa-K commented Jan 29, 2025

Summary

While creating a degraphql-route using deck, we were specifying
service field as a string. This is different than how deck usually
handles references. So, inorder to comply with the current deck
format, we are changing the service reference to a dict.

Full changelog

  • fix: changed dql routes format to show dict for service.
  • tests: fixed test files based on new dql-routes format.
  • fix: added validations to ensure correct format for configs is passed.
  • tests: added tests to handle for required fields/format.

Issues resolved

Pre-req for Kong/deck#1505

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

@codecov-commenter
Copy link

codecov-commenter commented Jan 29, 2025

Codecov Report

Attention: Patch coverage is 32.69231% with 35 lines in your changes missing coverage. Please review.

Project coverage is 28.17%. Comparing base (2eaa1ed) to head (ca3ddae).

Files with missing lines Patch % Lines
pkg/file/types.go 0.00% 32 Missing ⚠️
pkg/file/builder.go 85.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #187      +/-   ##
==========================================
+ Coverage   28.15%   28.17%   +0.02%     
==========================================
  Files         109      109              
  Lines       17190    17227      +37     
==========================================
+ Hits         4839     4854      +15     
- Misses      11830    11852      +22     
  Partials      521      521              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Prashansa-K Prashansa-K requested a review from GGabriele January 29, 2025 10:05
@Prashansa-K Prashansa-K marked this pull request as ready for review January 29, 2025 10:05
@Prashansa-K
Copy link
Collaborator Author

Before this change, the yaml format for custom_entities was as such:

custom_entities:
  - type: degraphql_routes
    fields:
      uri: "/foo"
      query: "query{ foo { bar } }"
      service: svc1

After this change, the format is as such:

custom_entities:
  - type: degraphql_routes
    fields:
      uri: "/foo"
      query: "query{ foo { bar } }"
      service:
        name: svc1

@Prashansa-K
Copy link
Collaborator Author

deck gateway dump would render the following:

custom_entities:
- fields:
    methods:
    - GET
    query: query{ foo { bar } }
    service:
      id: 870a8b7e-4885-4fcc-95d2-4d3dc75f1fe6
      name: svc1
    uri: /foo
  id: a6c032f6-4e8f-4dae-a351-5f1b6261b358
  type: degraphql_routes

@Prashansa-K Prashansa-K merged commit 8fdff5f into main Jan 31, 2025
24 checks passed
@Prashansa-K Prashansa-K deleted the fix/dql-routes-format branch January 31, 2025 11:09
assert.ErrorContains(t, err, "service field should be a map")
})

t.Run("create degraphql route - fails if service reference is not an object", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Prashansa-K this test is repeated? 👀

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My bad! I will clean this up.

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.

4 participants