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

[TT-162] Fix JSVM session metadata usage #3231

Conversation

matiasinsaurralde
Copy link
Contributor

Fix for #3218.

Description

When use_session is enabled, the SessionMeta object is expected to contain a content with the following structure:

{"tyk_developer_id":"5f1937ad4ea4611eefb8ede6","tyk_key_request_fields":{},"tyk_user_fields":{}}

Because we were previously using map[string]string, the key request and user fields break the unmarshalling step.
Using map[string]interface{} fixes the issue.

Related Issue

#3218.

How This Has Been Tested

Steps described in #3218.

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If pulling from your own
    fork, don't request your master!
  • Make sure you are making a pull request against the master branch (left side). Also, you should start
    your branch off our latest master.
  • My change requires a change to the documentation.
    • If you've changed APIs, describe what needs to be updated in the documentation.
    • If new config option added, ensure that it can be set via ENV variable
  • I have updated the documentation accordingly.
  • Modules and vendor dependencies have been updated; run go mod tidy && go mod vendor
  • When updating library version must provide reason/explanation for this update.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • Check your code additions will not fail linting checks:
    • go fmt -s
    • go vet

@buger
Copy link
Member

buger commented Jul 23, 2020

Can we add simple test for this?

@matiasinsaurralde matiasinsaurralde changed the title Fix JSVM session metadata usage [TT-162] Fix JSVM session metadata usage Sep 3, 2020
@matiasinsaurralde
Copy link
Contributor Author

Added tests and rebased the changes 👍

@matiasinsaurralde matiasinsaurralde force-pushed the virtual-endpoint-session-metadata branch 2 times, most recently from 8da590d to 91a7619 Compare October 16, 2020 20:08
Copy link
Contributor

@tbuchaillot tbuchaillot left a comment

Choose a reason for hiding this comment

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

LGTM

@matiasinsaurralde matiasinsaurralde merged commit 3c1de7b into TykTechnologies:master Oct 18, 2020
@matiasinsaurralde
Copy link
Contributor Author

/release to release-3-lts

@tykbot
Copy link

tykbot bot commented Oct 19, 2020

Working on it! Note that it can take a few minutes.

tykbot bot pushed a commit that referenced this pull request Oct 19, 2020
* mw_virtual_endpoint: use map[string]interface{} for JSVM session metadata

* mw_virtual_endpoint: improve test helper and session metadata test coverage

(cherry picked from commit 3c1de7b)
@tykbot
Copy link

tykbot bot commented Oct 19, 2020

@matiasinsaurralde Succesfully merged 3c1de7b1a2bcafb9ca7ad8f693398768558e80b2 to release-3-lts branch.

@matiasinsaurralde
Copy link
Contributor Author

/release to release-3.1

@tykbot
Copy link

tykbot bot commented Oct 20, 2020

Working on it! Note that it can take a few minutes.

@tykbot
Copy link

tykbot bot commented Oct 20, 2020

@matiasinsaurralde Seems like there is conflict and it require manual merge.

@tomshy
Copy link

tomshy commented Mar 4, 2021

Am testing with v3.1.2 but I think this issue is partially fixed. Getting Failed to decode middleware request data on return from VM when the session meta object contains

"tyk_user_fields":{}

I suspect it's because of the struct declaration on line 56

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