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

UI: remove default setting for max_versions in kv metadata #22394

Merged
merged 2 commits into from
Aug 17, 2023

Conversation

hellobontempo
Copy link
Contributor

@hellobontempo hellobontempo commented Aug 17, 2023

The default value of a secret's metadata was incorrectly setting max_versions to 10 which caused unintended side effects. This PR fixes #8656. Instead of setting the metadata's max_versions from the engine's config, leaving unset has the same result without manipulating any values under the hood.

The greater number takes precedence if both the engine and secret metadata have max_versions configured. Otherwise, 0 means unset and if neither have a max_versions value, Vault will keep the 10 latest versions.

global (engine) secret metadata *versions kept - why?!
0 0 But are unset so Vault keeps 10 versions
0 2* The engine has no global max_versions so the metadata value is used
8* 0 Metadata max_versions is unset so the engine's value is used
3 5* Metadata has a higher max_versions number
9* 2 Yep, you guessed it! The engine config is higher

Both metadata READ responses below have the same kv engine config:

# curl kv/config
=> "data": {
    "cas_required": false,
    "delete_version_after": "0s",
    "max_versions": 3
  }
# curl  kv/metadata/max-2
 => "data": {
    "cas_required": false,
    "created_time": "2023-08-17T03:29:14.58914Z",
    "current_version": 4,
    "custom_metadata": null,
    "delete_version_after": "0s",
    "max_versions": 2, # max_versions is 2 but below 3 are kept because that's the engine config
    "oldest_version": 2,
    "updated_time": "2023-08-17T03:31:09.852191Z",
    "versions": {
      "2": {
        "created_time": "2023-08-17T03:29:23.621884Z",
        "deletion_time": "",
        "destroyed": false
      },
      "3": {
        "created_time": "2023-08-17T03:30:58.966954Z",
        "deletion_time": "",
        "destroyed": false
      },
      "4": {
        "created_time": "2023-08-17T03:31:09.852191Z",
        "deletion_time": "",
        "destroyed": false
      }
    }
  }

# curl kv/metadata/max-5
 => "data": {
    "cas_required": false,
    "created_time": "2023-08-17T03:35:15.076621Z",
    "current_version": 6,
    "custom_metadata": null,
    "delete_version_after": "0s",
    "max_versions": 5, # even though the engine max_versions is 3, the metadata max is used because it's greater
    "oldest_version": 2,
    "updated_time": "2023-08-17T03:35:26.416381Z",
    "versions": {
      "2": {
        "created_time": "2023-08-17T03:35:17.780665Z",
        "deletion_time": "",
        "destroyed": false
      },
      "3": {
        "created_time": "2023-08-17T03:35:20.729444Z",
        "deletion_time": "",
        "destroyed": false
      },
      "4": {
        "created_time": "2023-08-17T03:35:22.889623Z",
        "deletion_time": "",
        "destroyed": false
      },
      "5": {
        "created_time": "2023-08-17T03:35:24.658798Z",
        "deletion_time": "",
        "destroyed": false
      },
      "6": {
        "created_time": "2023-08-17T03:35:26.416381Z",
        "deletion_time": "",
        "destroyed": false
      }
    }

Docs for engine max_versions

@hellobontempo hellobontempo added this to the 1.15 milestone Aug 17, 2023
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Aug 17, 2023
@github-actions
Copy link

Build Results:
All builds succeeded! ✅

@github-actions
Copy link

CI Results:
All Go tests succeeded! ✅

Copy link
Contributor

@Monkeychip Monkeychip left a comment

Choose a reason for hiding this comment

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

Thanks for tackling. Backport as far as to 1.12?

Copy link
Contributor

@hashishaw hashishaw left a comment

Choose a reason for hiding this comment

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

Thank you for the detailed description!

@hellobontempo hellobontempo merged commit 2c6a3e7 into main Aug 17, 2023
106 checks passed
@hellobontempo hellobontempo deleted the ui/VAULT-16815/fix-max-versions-default branch August 17, 2023 16:47
@hellobontempo hellobontempo modified the milestones: 1.15, 1.12.10 Aug 17, 2023
rebwill pushed a commit that referenced this pull request Aug 18, 2023
* remove default setting for max versions - leave unset with a value of 0

* add changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Max versions in the UI does not load default from secrets/config
3 participants