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

Enables a stateless version of Tyk Gateway #6457

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

lonelycode
Copy link
Member

@lonelycode lonelycode commented Aug 12, 2024

User description

This PR uses the new "local" storage driver and is dependent on the in-mem-kv branch/PR in the storage repo to enable Tyk to run without Redis.

Description

This PR enables a new "local" storage type configuration option which swaps the Redis driver to a local non-locking hash map. It also mocks the Queue interface to provide error-free pubsub (obviously it won't work in an actual cluster).

To test it, simply disable analytics, and set the storage type from "redis" to "local".

The local store does all complex operations in-memory, and does not rely on any underlying k/v data store capability (which means it could be used for more data stores even if they don't support Lists, Sets, or Sorted Sets).

Related Issue

JIRA Issue
Related PR

Motivation and Context

Providing a stateless gateway that can be deployed without a dependency makes time-to-initial-value faster for end-users. As it supports in-memory operations, quotas, rate-limits, and local tokens all work without any change. This is also useful for our own developers running tests, and existing customers that are spinning up test environments for their own engineers.

How This Has Been Tested

Manually tested the gateway (rate limit and quota only)
Underlying library passes all storage tests (keyvalue, set, sortedset, list, and queue)

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

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

PR Type

Enhancement


Description

  • Added support for a new "local" storage type configuration option which swaps the Redis driver to a local non-locking hash map.
  • Modified gateway initialization to handle "local" storage type for analytics and general storage connection.
  • Updated NewConnector function to handle "local" storage type.
  • Added a replacement directive in go.mod for the local storage module.

Changes walkthrough 📝

Relevant files
Enhancement
server.go
Add support for "local" storage type in gateway initialization

gateway/server.go

  • Added condition to support "local" storage type for analytics.
  • Modified error message to include "local" storage type.
  • +3/-3     
    connection_handler.go
    Handle "local" storage type in NewConnector function         

    storage/connection_handler.go

  • Added condition to handle "local" storage type in NewConnector
    function.
  • +5/-0     
    Configuration changes
    go.mod
    Add local replacement directive for storage module             

    go.mod

    • Added replacement directive for local storage module.
    +2/-0     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link
    Contributor

    API Changes

    no api changes detected

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Configuration Validation
    The error message in line 1252 is misleading as it only mentions Redis, despite the code now supporting 'local' storage type. This could confuse users about the valid configuration options.

    Connector Initialization
    The implementation for the 'local' storage type in line 243 does not handle potential errors that might occur during the creation of a new connector. It assumes success without any error checking.

    Copy link
    Contributor

    github-actions bot commented Aug 12, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Handle potential nil pointer dereference by checking if cfg is nil

    Consider handling the error case where cfg.Type might not be set, which could lead
    to a nil pointer dereference.

    storage/connection_handler.go [242-243]

    -if cfg.Type == "local" {
    +if cfg == nil || cfg.Type == "local" {
       return connector.NewConnector(model.LocalType)
     }
     
    Suggestion importance[1-10]: 10

    Why: This suggestion addresses a potential bug by preventing a nil pointer dereference, which is crucial for the stability and reliability of the code.

    10
    Enhancement
    Add a conditional check for analytics enablement before storage type validation

    Add a check to ensure that analytics are enabled before checking the storage type,
    to avoid unnecessary log errors when analytics are disabled.

    gateway/server.go [338-339]

    -if gwConfig.EnableAnalytics && gwConfig.Storage.Type != "redis" && gwConfig.Storage.Type != "local" {
    -  mainLog.Fatal("Analytics requires Redis Storage backend, please enable Redis in the tyk.conf file.")
    +if gwConfig.EnableAnalytics {
    +  if gwConfig.Storage.Type != "redis" && gwConfig.Storage.Type != "local" {
    +    mainLog.Fatal("Analytics requires Redis Storage backend, please enable Redis in the tyk.conf file.")
    +  }
     }
     
    Suggestion importance[1-10]: 9

    Why: Adding the conditional check ensures that the storage type validation is only performed when analytics are enabled, preventing unnecessary log errors and improving code efficiency.

    9
    Improve the error message for unsupported storage types

    Consider adding a more specific error message for when neither 'redis' nor 'local'
    storage types are configured. This will help users understand that the system
    supports only these two types and that one of them must be configured correctly.

    gateway/server.go [1251-1252]

     if gwConfig.Storage.Type != "redis" && gwConfig.Storage.Type != "local" {
    -  mainLog.Fatal("storage connection details not set, please ensure that the storage type is set to Redis and that the connection parameters are correct.")
    +  mainLog.Fatal("Unsupported storage type configured. Please ensure that the storage type is set to either 'redis' or 'local' and that the connection parameters are correct.")
     }
     
    Suggestion importance[1-10]: 7

    Why: The suggestion improves the clarity of the error message, making it more specific about the supported storage types. This enhances user understanding and debugging.

    7
    Maintainability
    Refactor storage type validation into a separate function

    Refactor the condition to check the storage type to a separate function to improve
    code readability and maintainability.

    gateway/server.go [1251-1252]

    -if gwConfig.Storage.Type != "redis" && gwConfig.Storage.Type != "local" {
    +if !isValidStorageType(gwConfig.Storage.Type) {
       mainLog.Fatal("storage connection details not set, please ensure that the storage type is set to Redis and that the connection parameters are correct.")
     }
     
    +// New function
    +func isValidStorageType(storageType string) bool {
    +  return storageType == "redis" || storageType == "local"
    +}
    +
    Suggestion importance[1-10]: 8

    Why: Refactoring the storage type validation into a separate function improves code readability and maintainability, making the codebase easier to manage and extend.

    8

    Copy link

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant