search

hashicorp / vault-plugin-secrets-mongodbatlas

Mozilla Public License 2.0
5 stars 3 forks source link

Noticing many "older than TTL" API Keys without projects assigned. #27

Closed Freyert closed 1 year ago

Freyert commented 2 years ago

We've identified that there is an error in the provider logic where the provider will:

  1. Create an API Key
  2. Fail to assign the API Key to a project

This causes the provider to exit with an error, but not clean up the API Key. If left unchecked you can easily exceed your organization's limit of 500 API Keys.

The worst part is that there is no record in Vault of the API Keys because it believes they were not created.

It may be that more nuanced control of the WAL is required so that the plugin can rollback operations that did not complete instead of just committing them in the error case.

See framework.PutWAL

PutWAL writes some data to the WAL.

The kind parameter is used by the framework to allow users to store multiple kinds of WAL data and to easily disambiguate what data they're expecting.

Data within the WAL that is uncommitted (CommitWAL hasn't be called) will be given to the rollback callback when an rollback operation is received, allowing the backend to clean up some partial states.

Freyert commented 2 years ago

A rollback function does exist, so perhaps the issue is there.

We're not seeing any logs either...

themantissa commented 1 year ago

@Freyert I'm highlighting this to the team at Hashi to see if they can take a look. In the mean time, and I know there's been a delay, do you have any more details? These were keys you were specifically creating for a project, not org level keys, and they just couldn't get assigned to their project but didn't fail cleanly? Just want to make sure we have the info needed to repro it.

austingebauer commented 1 year ago

@Freyert - Can you confirm that this (secret_programmatic_api_keys.go#L69) is the error your Vault client receives? I think the bug here is that we're immediately deleting the WAL entry. Instead, we should retain the WAL entry. It will eventually be picked up by the rollback function to clean up the partial failure.

Freyert commented 1 year ago

@themantissa @austingebauer I'm just coming back to this after a while. It's coming up on our list of things to do.

@themantissa I don't think I ever realized I could make API keys at project level! I just checked and these are ORG level API Keys, that are then added to projects.

We probably need to reevaluate these keys. I think the reason they are ORG keys is so that the key can list projects or something, but if you can only access one project then listing isn't really needed.

I think in general these teams would prefer to be able to request 1 API Key that allowed them to list projects, and then read from them. The processes that use these keys generally are just gathering information across a wide number of projects.

@austingebauer I'm not sure we ever found logs showing that API Keys failed to be invited to projects. In our issue notes I only see that we started failing to be able to create new API Keys because we exceeded our org limit.

Error creating programmatic api key: POST https://cloud.mongodb.com/api/atlas/v1.0/orgs/************************/apiKeys: 400 (request "MAX_API_KEYS_PER_ORG_EXCEEDED") Maximum number of API keys per org (500) in ************************ exceeded while trying to add API keys.,