Convert SubscriptionHistory create to an allowlist
What does this MR do and why?
Addresses: #360628 (closed)
This MR:
- Converts the
GitlabSubscriptionHistory
creation to use anallowlist
of attributes rather than adenylist
- Moves the creation logic for the history records to the history object
Prior to this change, if an attribute was added to the omit list and not tracked in the history table, it would cause issues with saving all GitlabSubscription
records on the main
stage. This is because the changes to the database are deployed across both stages, but the code changes are only deployed to the canary
stage first. This means that the main
stage is un-aware that it needs to omit the new attribute, as can be seen with this error here: https://sentry.gitlab.net/gitlab/gitlabcom/issues/3275019/?environment=gprd.
Converting this to an allowlist instead means that any new attributes will be ignored until the code hits the main
deployment stage as well.
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
- Create a new field on the
GitlabSubscription
tablerails g migration AddTestFieldToGitlabSubscriptions test_field:boolean && rails db:migrate
- Run the spec, it should fail as the attribute is not in a known attribute constant
rspec ee/spec/models/gitlab_subscription_spec.rb
- Make changes to a subscription in the console - it should still be able to save
subscription = GitlabSubscription.create!(namespace: Group.last) subscription.update!(test_field: false) # this should save successfully. If you repeat the process on master, this will error. GitlabSubscriptionHistory.last # should be the history logged for the previous change
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.