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

feat(api): Add configuration live update support #181

Merged
merged 3 commits into from
Apr 10, 2024

Conversation

rajdip-b
Copy link
Member

@rajdip-b rajdip-b commented Apr 10, 2024

User description

Description

Give a summary of the change that you have made

Fixes #[ISSUENO]

Dependencies

Mention any dependencies/packages used

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Add screenshots of relevant screens

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

Type

enhancement, bug_fix


Description

  • Integrated Redis and WebSocket support for real-time updates.
  • Added new ProviderModule and SocketModule for Redis and WebSocket functionality.
  • Implemented WebSocket gateway for change notifications.
  • Added Redis adapter for Socket.IO.
  • Updated Docker Compose configurations to include Redis service.
  • Added new dependencies for WebSocket and Redis support.
  • Updated test scripts to separate unit and E2E tests.

Changes walkthrough

Relevant files
Configuration changes
4 files
jest.e2e-config.ts
Add Force Exit to Jest E2E Configuration                                 

apps/api/jest.e2e-config.ts

  • Added forceExit: true to Jest E2E configuration.
+1/-0     
docker-compose-test.yml
Add Redis Service to Docker Compose Test Configuration     

docker-compose-test.yml

  • Added Redis service configuration for testing environment.
+6/-0     
docker-compose.yml
Add Redis Service to Docker Compose Dev Configuration       

docker-compose.yml

  • Added Redis service configuration for development environment.
+9/-3     
package.json
Update Test Scripts for Separate Unit and E2E Tests           

package.json

  • Updated test scripts to separate unit and E2E tests.
+2/-1     
Enhancement
11 files
app.module.ts
Integrate Socket, Provider, and Schedule Modules into AppModule

apps/api/src/app/app.module.ts

  • Imported SocketModule, ProviderModule, and ScheduleModule into the
    AppModule.
  • +7/-1     
    auth.guard.ts
    Enhance Auth Guard with WebSocket Support and Strict Auth Checks

    apps/api/src/auth/guard/auth/auth.guard.ts

  • Added a check to throw ForbiddenException if no authentication is
    provided outside of e2e tests.
  • Adjusted to support WebSocket headers in addition to HTTP headers.
  • +15/-9   
    environment.ts
    Introduce Environment Constants for Env Variables               

    apps/api/src/common/environment.ts

  • Introduced Environment constant for managing environment variables.
  • +3/-0     
    main.ts
    Integrate RedisIoAdapter and Disable CORS                               

    apps/api/src/main.ts

    • Disabled CORS and integrated RedisIoAdapter for WebSocket support.
    +7/-1     
    provider.module.ts
    Create Provider Module for Redis Client                                   

    apps/api/src/provider/provider.module.ts

    • Created a new ProviderModule for Redis client provisioning.
    +14/-0   
    redis.provider.ts
    Implement Redis Provider with Pub/Sub Clients                       

    apps/api/src/provider/redis.provider.ts

    • Implemented Redis provider with publisher and subscriber clients.
    +40/-0   
    change-notifier.socket.ts
    Implement WebSocket Gateway for Change Notifications         

    apps/api/src/socket/change-notifier.socket.ts

  • Implemented a WebSocket gateway for change notifications with Redis
    integration.
  • +231/-0 
    redis.adapter.ts
    Implement Redis Adapter for Socket.IO                                       

    apps/api/src/socket/redis.adapter.ts

  • Implemented a Redis adapter for Socket.IO to support WebSocket
    communication.
  • +26/-0   
    socket.module.ts
    Create Socket Module for WebSocket Functionality                 

    apps/api/src/socket/socket.module.ts

    • Created a new SocketModule for WebSocket-related functionality.
    +7/-0     
    migration.sql
    Add SQL Migration for ChangeNotificationSocketMap Table   

    apps/api/src/prisma/migrations/20240410130019_add_change_notification_socket_map/migration.sql

  • Added a new SQL migration for creating ChangeNotificationSocketMap
    table.
  • +11/-0   
    schema.prisma
    Add ChangeNotificationSocketMap Model to Prisma Schema     

    apps/api/src/prisma/schema.prisma

    • Added ChangeNotificationSocketMap model to Prisma schema.
    +8/-0     
    Bug fix
    1 files
    e2e.setup.ts
    Remove E2E Setup Class                                                                     

    apps/api/src/app/e2e.setup.ts

    • Removed the E2ESetup class entirely.
    +0/-57   
    Tests
    1 files
    approval.controller.spec.ts
    Update Approval Controller Tests with Redis Mock                 

    apps/api/src/approval/controller/approval.controller.spec.ts

  • Added ProviderModule to imports.
  • Mocked REDIS_CLIENT provider.
  • +9/-1     
    Dependencies
    1 files
    package.json
    Add Dependencies for WebSocket and Redis Support                 

    apps/api/package.json

    • Added dependencies for WebSocket and Redis support.
    +5/-0     

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

    @rajdip-b rajdip-b changed the title https://github.com/keyshade-xyz/keyshade/pull/new/feat/realtime-updateshttps://github.com/keyshade-xyz/keyshade/pull/new/feat/realtime-updatesfeat: Add realtime update feature feat(api): Add live configuration update feature Apr 10, 2024
    @codiumai-pr-agent-free codiumai-pr-agent-free bot changed the title feat(api): Add live configuration update feature https://github.com/keyshade-xyz/keyshade/pull/new/feat/realtime-updateshttps://github.com/keyshade-xyz/keyshade/pull/new/feat/realtime-updatesfeat: Add realtime update feature Apr 10, 2024
    Copy link
    Contributor

    PR Description updated to latest commit (782f35e)

    Copy link
    Contributor

    PR Review

    ⏱️ Estimated effort to review [1-5]

    4, due to the extensive changes across multiple files, including new modules, updates to existing services, and integration with Redis and WebSocket. The addition of new functionality and its integration with existing components increases the complexity and the time required to thoroughly review the changes.

    🧪 Relevant tests

    Yes

    🔍 Possible issues

    Possible Bug: The removal of apps/api/src/app/e2e.setup.ts might affect existing E2E tests that rely on the setup provided by this file. Ensure that all E2E tests are updated accordingly to work without this setup.

    Performance Concern: The use of Redis for real-time updates and the subscription model in apps/api/src/socket/change-notifier.socket.ts could lead to performance issues if not properly optimized, especially with a large number of concurrent connections or notifications.

    Dependency Management: The addition of new dependencies (redis, @socket.io/redis-adapter, etc.) requires careful management to ensure they are kept up to date and do not introduce security vulnerabilities.

    🔒 Security concerns

    No


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link
    Contributor

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Best practice
    Improve type safety by specifying a more precise type than any[].

    Consider using a more specific type than any[] for the args parameter in the
    handleConnection method to improve type safety and code readability.

    apps/api/src/socket/change-notifier.socket.ts [78]

    -async handleConnection(client: Socket, ...args: any[]) {
    +async handleConnection(client: Socket, ...args: CustomType[]) {
     
    Ensure data consistency by using a transaction for related operations.

    Use a transaction for operations that should be executed together to ensure data
    consistency in the handleRegister method.

    apps/api/src/socket/change-notifier.socket.ts [104-134]

    -await this.prisma.workspace.findFirst({
    -  where: {
    -    name: data.workspaceName
    -  }
    -})
    +const transaction = await this.prisma.$transaction([
    +  this.prisma.workspace.findFirst({
    +    where: {
    +      name: data.workspaceName
    +    }
    +  }),
    +  this.prisma.project.findFirst({
    +    where: {
    +      name: data.projectName
    +    }
    +  }),
    +  this.prisma.environment.findFirst({
    +    where: {
    +      name: data.environmentName
    +    }
    +  })
    +])
     
    Specify explicit return type for createIOServer.

    It's recommended to explicitly define the return type of the createIOServer method to
    improve code readability and maintainability. This helps developers understand what the
    method is expected to return without diving into the implementation details.

    apps/api/src/socket/redis.adapter.ts [21]

    -createIOServer(port: number, options?: ServerOptions): any {
    +createIOServer(port: number, options?: ServerOptions): Server {
     
    Enable TypeScript compiler options for better code safety and maintainability.

    It's recommended to enable forceConsistentCasingInFileNames and noFallthroughCasesInSwitch
    in your TypeScript configuration for better code safety and maintainability.
    forceConsistentCasingInFileNames ensures that the casing of your file names is consistent
    across your codebase, which is important for working across different operating systems.
    noFallthroughCasesInSwitch prevents fall-through behavior in switch statements, which can
    be a source of bugs.

    apps/api/tsconfig.json [18-19]

    -"forceConsistentCasingInFileNames": false,
    -"noFallthroughCasesInSwitch": false
    +"forceConsistentCasingInFileNames": true,
    +"noFallthroughCasesInSwitch": true
     
    Error handling
    Handle potential Redis publishing errors gracefully with a try-catch block.

    Consider using a try-catch block around the Redis publish operation to handle potential
    errors gracefully.

    apps/api/src/variable/service/variable.service.ts [545-552]

    -await this.redis.publish(
    -  CHANGE_NOTIFIER_RSC,
    -  JSON.stringify({
    -    environmentId: variable.environmentId,
    -    name: variable.name,
    -    value: dto.value,
    -    isSecret: false
    -  })
    -)
    +try {
    +  await this.redis.publish(
    +    CHANGE_NOTIFIER_RSC,
    +    JSON.stringify({
    +      environmentId: variable.environmentId,
    +      name: variable.name,
    +      value: dto.value,
    +      isSecret: false
    +    })
    +  )
    +} catch (error) {
    +  this.logger.error('Failed to publish update to Redis', error)
    +}
     
    Add error handling for Redis connection failures to improve resilience.

    Consider adding error handling for Redis connection failures to prevent the application
    from exiting immediately, allowing for retries or logging.

    apps/api/src/provider/redis.provider.ts [28]

    -await publisher.connect()
    +publisher.connect().catch((error) => {
    +  logger.error('Failed to connect to Redis publisher:', error)
    +  // Implement retry logic or other error handling here
    +})
     
    Maintainability
    Reduce code duplication by extracting repeated header retrieval logic into a method.

    Extract the repeated logic for getting headers from a request into a separate method to
    reduce duplication and improve maintainability.

    apps/api/src/auth/guard/auth/auth.guard.ts [136-153]

    -const headers = request.headers || request.handshake.headers // For websockets
    +private getHeaders(request: any): any {
    +  return request.headers || request.handshake.headers // For websockets
    +}
     
    Use dynamic module imports for conditional loading.

    To improve the modularity and maintainability of your application, consider using dynamic
    module imports for SocketModule and ProviderModule if they depend on external
    configurations or need to be conditionally loaded. This approach can help in keeping the
    AppModule cleaner and more scalable.

    apps/api/src/app/app.module.ts [46-47]

    -SocketModule,
    -ProviderModule
    +SocketModule.forRoot({ /* configuration options */ }),
    +ProviderModule.forRoot({ /* configuration options */ })
     
    Enhancement
    Add error handling for Redis connection.

    Consider handling potential errors when connecting to Redis. This can be done by wrapping
    the connection logic in a try-catch block and logging or handling the error appropriately.
    This will ensure that your application can gracefully handle situations where the Redis
    server is unavailable or the connection details are incorrect.

    apps/api/src/socket/redis.adapter.ts [16]

    -await Promise.all([pubClient.connect(), subClient.connect()])
    +try {
    +  await Promise.all([pubClient.connect(), subClient.connect()])
    +} catch (error) {
    +  console.error('Failed to connect to Redis:', error)
    +  // Handle the error appropriately
    +}
     
    Handle non-existent variableId in updateVariable.

    Ensure that the updateVariable method properly handles the case where the variableId does
    not exist in the database. This can be done by checking the result of the updateVariable
    service call and throwing a NotFoundException if the variable is not found.

    apps/api/src/variable/controller/variable.controller.ts [48]

    -return await this.variableService.updateVariable(
    +const updatedVariable = await this.variableService.updateVariable(
    +if (!updatedVariable) {
    +  throw new NotFoundException(`Variable with ID ${variableId} not found`);
    +}
    +return updatedVariable;
     
    Add caching for node modules to speed up workflow execution.

    Consider adding a step to cache the node modules to speed up the workflow execution. This
    can be achieved by using actions like actions/cache before running pnpm run
    db:generate-types and pnpm run unit:api. This will reduce the time taken to install
    dependencies if they haven't changed.

    .github/workflows/validate-api.yaml [53-54]

    -pnpm run db:generate-types
    -pnpm run unit:api
    +- name: Cache node modules
    +  uses: actions/cache@v2
    +  with:
    +    path: ~/.pnpm-store
    +    key: ${{ runner.os }}-node-${{ hashFiles('**/pnpm-lock.yaml') }}
    +    restore-keys: |
    +      ${{ runner.os }}-node-
    +- name: Unit tests
    +  run: |
    +    pnpm run db:generate-types
    +    pnpm run unit:api
     
    Security
    Enable and configure CORS for better security.

    For better security practices, consider enabling CORS with specific configurations instead
    of disabling it entirely. Disabling CORS can expose your application to cross-origin
    resource sharing issues. You can configure CORS to allow requests only from trusted
    origins.

    apps/api/src/main.ts [83]

    -cors: false
    +cors: {
    +  origin: 'https://yourtrustedorigin.com',
    +  methods: 'GET,HEAD,PUT,PATCH,POST,DELETE',
    +  preflightContinue: false,
    +  optionsSuccessStatus: 204
    +}
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    @rajdip-b rajdip-b changed the title https://github.com/keyshade-xyz/keyshade/pull/new/feat/realtime-updateshttps://github.com/keyshade-xyz/keyshade/pull/new/feat/realtime-updatesfeat: Add realtime update feature feat(api): Add configuration live update support Apr 10, 2024
    Copy link

    sonarcloud bot commented Apr 10, 2024

    Quality Gate Passed Quality Gate passed

    Issues
    0 New issues
    0 Accepted issues

    Measures
    0 Security Hotspots
    No data about Coverage
    0.0% Duplication on New Code

    See analysis details on SonarCloud

    Copy link

    codecov bot commented Apr 10, 2024

    Codecov Report

    Attention: Patch coverage is 67.39130% with 45 lines in your changes are missing coverage. Please review.

    Project coverage is 93.10%. Comparing base (7bb3d21) to head (f131958).
    Report is 43 commits behind head on develop.

    Files Patch % Lines
    apps/api/src/socket/change-notifier.socket.ts 52.17% 33 Missing ⚠️
    apps/api/src/auth/guard/auth/auth.guard.ts 73.33% 4 Missing ⚠️
    apps/api/src/provider/redis.provider.ts 78.94% 4 Missing ⚠️
    apps/api/src/secret/service/secret.service.ts 83.33% 2 Missing ⚠️
    apps/api/src/variable/service/variable.service.ts 84.61% 2 Missing ⚠️
    Additional details and impacted files
    @@             Coverage Diff              @@
    ##           develop     #181       +/-   ##
    ============================================
    + Coverage    62.20%   93.10%   +30.89%     
    ============================================
      Files           76       96       +20     
      Lines         1503     2131      +628     
      Branches       260      399      +139     
    ============================================
    + Hits           935     1984     +1049     
    + Misses         568      147      -421     
    Flag Coverage Δ
    api-e2e-tests 93.10% <67.39%> (+30.89%) ⬆️

    Flags with carried forward coverage won't be shown. Click here to find out more.

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

    @rajdip-b rajdip-b merged commit f7d6684 into develop Apr 10, 2024
    6 of 7 checks passed
    @rajdip-b rajdip-b deleted the feat/realtime-updates branch May 6, 2024 19:39
    rajdip-b pushed a commit that referenced this pull request May 12, 2024
    ## [1.3.0](v1.2.0...v1.3.0) (2024-05-12)
    
    ### 🚀 Features
    
    * Add approval support ([#158](#158)) ([e09ae60](e09ae60))
    * **api:** Add configuration live update support ([#181](#181)) ([f7d6684](f7d6684))
    * **api:** Add feature to export data of a workspace ([#152](#152)) ([46833aa](46833aa))
    * **api:** Add Integration support ([#203](#203)) ([f1ae87e](f1ae87e))
    * **api:** Add note to [secure] and variable ([#151](#151)) ([2e62351](2e62351))
    * **api:** Add OAuth redirection and polished authentication ([#212](#212)) ([d2968bc](d2968bc))
    * **api:** Add support for storing and managing variables ([#149](#149)) ([963a8ae](963a8ae))
    * **api:** Added GitLab OAuth ([#188](#188)) ([4d3bbe4](4d3bbe4))
    * **api:** Added validation for reason field ([#190](#190)) ([90b8ff2](90b8ff2))
    * **api:** Create default workspace on user's creation ([#182](#182)) ([3dc0c4c](3dc0c4c))
    * **api:** Reading `port` Dynamically ([#170](#170)) ([fd46e3e](fd46e3e))
    * **auth:** Add Google OAuth ([#156](#156)) ([cf387ea](cf387ea))
    * **web:** Added waitlist ([#168](#168)) ([1084c77](1084c77))
    * **web:** Landing revamp ([#165](#165)) ([0bc723b](0bc723b))
    
    ### 🐛 Bug Fixes
    
    * **web:** alignment issue in “Collaboration made easy” section ([#178](#178)) ([df5ca75](df5ca75))
    * **workspace:** delete duplicate tailwind config ([99d922a](99d922a))
    
    ### 📚 Documentation
    
    * add contributor list ([f37569a](f37569a))
    * Add integration docs ([#204](#204)) ([406ddb7](406ddb7))
    * Added integration docs to gitbook summary ([ab37530](ab37530))
    * **api:** Add swagger docs of API key controller ([#167](#167)) ([2910476](2910476))
    * **api:** Add swagger docs of User Controller ([#166](#166)) ([fd59522](fd59522))
    * fix typo in environment-variables.md ([#163](#163)) ([48294c9](48294c9))
    * Remove supabase from docs ([#169](#169)) ([eddbce8](eddbce8))
    * **setup:** replace NX with Turbo in setup instructions ([#175](#175)) ([af8a460](af8a460))
    * Update README.md ([b59f16b](b59f16b))
    * Update running-the-api.md ([177dbbf](177dbbf))
    * Update running-the-api.md ([#193](#193)) ([3d5bcac](3d5bcac))
    
    ### 🔧 Miscellaneous Chores
    
    * Added lockfile ([60a3b9b](60a3b9b))
    * Added lockfile ([6bb512c](6bb512c))
    * **api:** Added type inference and runtime validation to `process.env` ([#200](#200)) ([249e07d](249e07d))
    * **api:** Fixed prisma script env errors ([#209](#209)) ([8762354](8762354))
    * **API:** Refactor authority check functions in API ([#189](#189)) ([e9d710d](e9d710d))
    * **api:** Refactor user e2e tests ([b38d45a](b38d45a))
    * **ci:** Disabled api stage release ([97877c4](97877c4))
    * **ci:** Update stage deployment config ([868a6a1](868a6a1))
    * **codecov:** update api-e2e project coverage ([1e90d7e](1e90d7e))
    * **dockerfile:** Fixed web dockerfile ([6134bb2](6134bb2))
    * **docker:** Optimized web Dockerfile to reduct image size ([#173](#173)) ([444286a](444286a))
    * **release:** Downgraded package version ([c173fee](c173fee))
    * **release:** Fix failing release ([#213](#213)) ([40f64f3](40f64f3))
    * **release:** Install pnpm ([1081bea](1081bea))
    * **release:** Updated release commit ([b8958e7](b8958e7))
    * **release:** Updated release commit ([e270eb8](e270eb8))
    * Update deprecated husky Install command ([#202](#202)) ([e61102c](e61102c))
    * Upgrade @million/lint from 0.0.66 to 0.0.73 ([#172](#172)) ([dd43ed9](dd43ed9))
    * **web:** Updated fly memory config ([4debc66](4debc66))
    
    ### 🔨 Code Refactoring
    
    * **api:** Made events central to workspace ([#159](#159)) ([9bc00ae](9bc00ae))
    * **api:** Migrated to cookie based authentication ([#206](#206)) ([ad6911f](ad6911f))
    * **monorepo:** Migrate from nx to turbo ([#153](#153)) ([88b4b00](88b4b00))
    @rajdip-b
    Copy link
    Member Author

    🎉 This PR is included in version 1.3.0 🎉

    The release is available on GitHub release

    Your semantic-release bot 📦🚀

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type: enhancement New feature or request
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant