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

Clean node resolver map entries on attested node deletion #3873

Merged
merged 4 commits into from
Feb 24, 2023

Conversation

guilhermocc
Copy link
Contributor

@guilhermocc guilhermocc commented Feb 16, 2023

Pull Request check list

  • Commit conforms to CONTRIBUTING.md?
  • Proper tests/regressions included?
  • Documentation updated?

Affected functionality

  • Agent eviction action

Description of change
Update DeleteAttestedNode method to also delete node selectors

Which issue this PR fixes
fixes #3872

Signed-off-by: Guilherme Carvalho <guilhermbrsp@gmail.com>
Signed-off-by: Guilherme Carvalho <guilhermbrsp@gmail.com>
Signed-off-by: Guilherme Carvalho <guilhermbrsp@gmail.com>
@MarcosDY MarcosDY added this to the 1.6.1 milestone Feb 21, 2023
Copy link
Member

@azdagron azdagron left a comment

Choose a reason for hiding this comment

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

Thanks, @guilhermocc! I wonder if we should open a separate issue to provide some database cleanup code that deletes selector rows that have a spiffe ID not represented in the attested nodes table, maybe when the database is first opened, that we could keep in for a minor release cycle to make sure stale entries are removed eventually.

@guilhermocc
Copy link
Contributor Author

Thanks, @guilhermocc! I wonder if we should open a separate issue to provide some database cleanup code that deletes selector rows that have a spiffe ID not represented in the attested nodes table, maybe when the database is first opened, that we could keep in for a minor release cycle to make sure stale entries are removed eventually.

It makes sense since this PR will only prevent the existence of new stale node resolver map entries, but will not clean existing ones. I can create it :)

@guilhermocc
Copy link
Contributor Author

@azdagron update about the created issue: #3919

@amartinezfayo amartinezfayo modified the milestones: 1.6.2, 1.6.3 Apr 5, 2023
stevend-uber pushed a commit to stevend-uber/spire that referenced this pull request Oct 16, 2023
Signed-off-by: Guilherme Carvalho <guilhermbrsp@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attested node deletion doesn't clean node resolver data
4 participants