Skip to content

Commit

Permalink
[TT-13390] Silently skip loading bundle on managment node (#6739)
Browse files Browse the repository at this point in the history
### **User description**
<details open>
<summary><a href="https://tyktech.atlassian.net/browse/TT-13390"
title="TT-13390" target="_blank">TT-13390</a></summary>
  <br />
  <table>
    <tr>
      <th>Summary</th>
<td>Security feature introduced in v5.3.1 (skip loading API) introduces
problems for Cloud users (OAuth APIs)</td>
    </tr>
    <tr>
      <th>Type</th>
      <td>
<img alt="Bug"
src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10303?size=medium"
/>
        Bug
      </td>
    </tr>
    <tr>
      <th>Status</th>
      <td>In Dev</td>
    </tr>
    <tr>
      <th>Points</th>
      <td>N/A</td>
    </tr>
    <tr>
      <th>Labels</th>
<td><a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20'24Bugsmash%20ORDER%20BY%20created%20DESC"
title="'24Bugsmash">'24Bugsmash</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20Adhoc%20ORDER%20BY%20created%20DESC"
title="Adhoc">Adhoc</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20jira_escalated%20ORDER%20BY%20created%20DESC"
title="jira_escalated">jira_escalated</a></td>
    </tr>
  </table>
</details>
<!--
  do not remove this marker as it will break jira-lint's functionality.
  added_by_jira_lint
-->

---

<!-- Provide a general summary of your changes in the Title above -->

## Description

<!-- Describe your changes in detail -->

## Related Issue
https://tyktech.atlassian.net/browse/TT-13390

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] 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

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the 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**
Bug fix, Tests


___

### **Description**
- Updated the `loadBundle` function to silently skip loading bundles
when the gateway instance is a management node.
- Clarified the usage of the `management_node` configuration in the
comments, specifying that it should always be set to `false` in pro
environments.
- Added a new test case to ensure that `loadBundle` behaves correctly
when the gateway is a management node.



___



### **Changes walkthrough** 📝
<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Documentation</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>config.go</strong><dd><code>Clarify `management_node`
configuration usage in comments</code></dd></summary>
<hr>

config/config.go

<li>Added clarification in the comments regarding the
<code>management_node</code> <br>configuration.<br> <li> Specified that
<code>management_node</code> should always be set to <code>false</code>
in pro <br>environments.<br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6739/files#diff-fe44f09c4d5977b5f5eaea29170b6a0748819c9d02271746a20d81a5f3efca17">+3/-0</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>
</table></td></tr><tr><td><strong>Bug fix</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>coprocess_bundle.go</strong><dd><code>Skip bundle
loading on management nodes in `loadBundle`</code>&nbsp; &nbsp;
</dd></summary>
<hr>

gateway/coprocess_bundle.go

<li>Updated <code>loadBundle</code> function to silently skip loading
the bundle if the <br>node is a management node.<br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6739/files#diff-72df0cbfd3765a5d0bff62196c11008596608a21a53dbb9c65bfc6f008dbfa29">+4/-0</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>
</table></td></tr><tr><td><strong>Tests</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>coprocess_bundle_test.go</strong><dd><code>Add test for
skipping bundle loading on management nodes</code>&nbsp; </dd></summary>
<hr>

gateway/coprocess_bundle_test.go

<li>Added a new test case to verify that <code>loadBundle</code> does
not load bundles <br>or throw errors when the gateway is a management
node.<br>


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6739/files#diff-7fded1570c90f7be73d562f3ebcbb32fe4d50548dc5e959d8ecadddef13941fa">+17/-0</a>&nbsp;
&nbsp; </td>

</tr>
</table></td></tr></tr></tbody></table>

___

> 💡 **PR-Agent usage**: Comment `/help "your question"` on any pull
request to receive relevant information

(cherry picked from commit 7fbd718)
  • Loading branch information
jeffy-mathew authored and Tyk Bot committed Dec 4, 2024
1 parent c26b2ac commit 1506e90
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 0 deletions.
3 changes: 3 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,9 @@ type Config struct {
// Note:
// If you set `db_app_conf_options.node_is_segmented` to `true` for multiple Gateway nodes, you should ensure that `management_node` is set to `false`.
// This is to ensure visibility for the management node across all APIs.
//
// For pro installations, `management_node` is not a valid configuration option.
// Always set `management_node` to `false` in pro environments.
ManagementNode bool `json:"management_node"`

// This is used as part of the RPC / Hybrid back-end configuration in a Tyk Enterprise installation and isn’t used anywhere else.
Expand Down
4 changes: 4 additions & 0 deletions gateway/coprocess_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,10 @@ func (gw *Gateway) getHashedBundleName(bundleName string) (string, error) {

// loadBundle wraps the load and save steps, it will return if an error occurs at any point.
func (gw *Gateway) loadBundle(spec *APISpec) error {
if gw.GetConfig().ManagementNode {
return nil
}

// Skip if no custom middleware bundle name is set.
if spec.CustomMiddlewareBundleDisabled || spec.CustomMiddlewareBundle == "" {
return nil
Expand Down
17 changes: 17 additions & 0 deletions gateway/coprocess_bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,23 @@ func TestBundleLoader(t *testing.T) {
assert.NoError(t, err)
})

t.Run("load bundle should not load bundle nor error when the gateway instance is a management node", func(t *testing.T) {
customTs := StartTest(func(globalConf *config.Config) {
globalConf.ManagementNode = true
})

t.Cleanup(customTs.Close)
spec := &APISpec{
APIDefinition: &apidef.APIDefinition{
CustomMiddlewareBundle: "some-bundle",
CustomMiddlewareBundleDisabled: false,
},
}
err := customTs.Gw.loadBundle(spec)
assert.Empty(t, spec.CustomMiddleware)
assert.NoError(t, err)
})

t.Run("Load bundle fails if public key path is set but no signature is provided", func(t *testing.T) {
cfg := ts.Gw.GetConfig()
cfg.PublicKeyPath = "random/path/to/public.key"
Expand Down

0 comments on commit 1506e90

Please sign in to comment.