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

Fix bug where attr_writer was overriding the key_root= method. #4097

Merged
merged 1 commit into from Aug 28, 2015

Conversation

jrafanie
Copy link
Member

Extracted from #4082

@kbrock please review...@Fryguy and I fixed an issue where the key_root= class method was being

✌️ whacked ✌️

by the attr_writer, causing the v*_key caches to not be reset via key_root=, particularly during tests.

I ran into this "bug" locally for seemingly unrelated tests and couldn't track it down previously:
bin/rspec gems/pending/spec/util/miq-password_spec.rb spec/models/miq_server/rhn_mirror_spec.rb --seed 46323

It's weird to run gems/pending tests with models but I was doing it for changes that affected all of miq_server and util.

This test would run the miq-password spec first, reset the key_root, not clear out the v*_key, and subsequent tests that run MiqDatabase.seed such as rhn_mirror_spec.rb would fail with seemingly cryptic errors like:

     Failure/Error: it("#decrypt")        { expect(MiqPassword.new.decrypt(enc_v2)).to           be_decrypted(pass) }
     OpenSSL::Cipher::CipherError:
       bad decrypt

Or

     Failure/Error: it("#recrypt legacy") { expect(MiqPassword.new.recrypt(enc_v0)).to eq(enc_v2) }

       expected: "v2:{27X41c6xqCCdVcw4LlQ1Qg==}"
            got: "v2:{kGuPkrN8fYq0cJPTjytisw==}"

These tests now don't randomly fail with this PR.

@jrafanie jrafanie force-pushed the fix_overridden_key_root_setter branch from 87ea6be to 6cbf5a4 Compare August 28, 2015 18:51
@miq-bot
Copy link
Member

miq-bot commented Aug 28, 2015

Checked commit jrafanie@6cbf5a4 with rubocop 0.33.0 and haml-lint 0.13.0
2 files checked, 0 offenses detected
Everything looks good. 🍰

kbrock added a commit that referenced this pull request Aug 28, 2015
Fix bug where attr_writer was overriding the key_root= method.
@kbrock kbrock merged commit 0336c3e into ManageIQ:master Aug 28, 2015
@chessbyte chessbyte modified the milestone: Sprint 29 Ending Sept 14, 2015 Aug 29, 2015
@jrafanie jrafanie deleted the fix_overridden_key_root_setter branch October 14, 2015 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants