Project

General

Profile

Bug #24640

1.17 migration causes array/hash values for parameters to turn into strings with escaped quotes

Added by Tomer Brisker 3 months ago. Updated 3 months ago.

Status:
Closed
Priority:
High
Assignee:
Category:
Smart Variables
Target version:

Description

Looks like an issue in db/migrate/20170112175131_migrate_template_to_parameters_macros.rb causes any such lookupkeys/lookupvalues to turn their values to strings.
Since the values are saved using update_attribute, callbacks aren't executed meaning cast_default_value is not called and the value remains a string.

For example:

#<VariableLookupKey:0x0000000015b50738
 id: 21532,
 key: "zdcaz",
 created_at: Thu, 16 Aug 2018 13:01:56 UTC +00:00,
 updated_at: Thu, 16 Aug 2018 13:01:56 UTC +00:00,
 puppetclass_id: 5,
 default_value: ["a", "b"],
 path: nil,
 description: "",
 validator_type: "",
 validator_rule: nil,
 key_type: "array",
 override: false,
 required: false,
 merge_overrides: false,
 avoid_duplicates: false,
 omit: nil,
 type: "VariableLookupKey",
 merge_default: false,
 hidden_value: false>
[152] pry(main)> k.default_value = convert(k.default_value.to_s)
=> "[\"a\", \"b\"]" 

As a workaround, it may be possible to iterate over all Lookupkeys and values and save them all again to ensure casting is called from foreman-rake console (please take care to backup your database before running this):

LookupKey.unscoped.find_each(&:save_without_auditing)


Related issues

Related to Foreman - Feature #16740: Host parameters should be available in templates using some macroClosed2016-09-29
Related to Foreman - Bug #23581: Upgrade to Foreman 1.17 converts YAML to JSON HashResolved
Has duplicate Foreman - Bug #23809: Unable to save overridden Array class smart parametersDuplicate

Associated revisions

Revision e719d906 (diff)
Added by Tomer Brisker 3 months ago

Fixes #24640 - Cast all lookup keys and values (#5956)

  • Fixes #24640 - Cast all lookup keys and values

db/migrate/20170112175131_migrate_template_to_parameters_macros.rb
caused all LookupKey default values and LookupValue values to be saved
as strings, because it updated the attributes without calling callbacks,
causing cast_default_value or cast_value callbacks to be ignored.
This causes any values not of string type to break.

Revision 705d8644 (diff)
Added by Tomer Brisker 3 months ago

Refs #24640 - Cast omitted default with merge_overrides

In case the default value is omitted and the merge_override option is
selcted, the previous version of this migration didn't cast the default
leading to a failure when merging overrides.
Error message was also improved to be clearer and less frightening than
a stack trace.

History

#1 Updated by Tomer Brisker 3 months ago

  • Related to Feature #16740: Host parameters should be available in templates using some macro added

#2 Updated by Tomer Brisker 3 months ago

  • Has duplicate Bug #23809: Unable to save overridden Array class smart parameters added

#3 Updated by The Foreman Bot 3 months ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/5956 added

#4 Updated by Tomer Brisker 3 months ago

  • Assignee changed from Ori Rabin to Tomer Brisker

#5 Updated by Tomer Brisker 3 months ago

  • Bugzilla link set to 1618378

#6 Updated by Marek Hulán 3 months ago

  • Fixed in Releases 1.20.0 added

#7 Updated by Tomer Brisker 3 months ago

  • Fixed in Releases 1.19.0 added

#8 Updated by Tomer Brisker 3 months ago

  • Status changed from Ready For Testing to Closed

#9 Updated by The Foreman Bot 3 months ago

  • Pull request https://github.com/theforeman/foreman/pull/5977 added

#10 Updated by Daniel Helgenberger 3 months ago

Even after the migration, I still have issues with some deep merges. 1.16.2 does render this override as expected:

  mb_linux::cloudflared:
    ensure: present
    settings:
      no-tls-verify: true
      no-chunked-encoding: true

After the upgrade to 1.18 it fails, even if I set the parameter type to hash.

#11 Updated by Daniel Helgenberger 3 months ago

The migration did fail because I had this value of type yaml in one of my parameters:

--- "/srv/pipeline 10.11.0.0/16(ro,root_squash) fdff:dd77:2c2c:1100::/56(ro,root_squash)" 
# foreman-rake db:migrate:redo VERSION=20180816134832
/usr/share/foreman/lib/core_extensions.rb:182: warning: already initialized constant ActiveSupport::MessageEncryptor::DEFAULT_CIPHER
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activesupport-5.1.6/lib/active_support/message_encryptor.rb:22: warning: previous definition of DEFAULT_CIPHER was here
== 20180816134832 CastLookupKeyValues: reverting ==============================
== 20180816134832 CastLookupKeyValues: reverted (0.0000s) =====================

== 20180816134832 CastLookupKeyValues: migrating ==============================
Error casting value /srv/pipeline 10.11.0.0/16(ro,root_squash) fdff:dd77:2c2c:1100::/56(ro,root_squash) for #<LookupValue id: 663, match: "fqdn=pipeline.int.m-box.de", value: "/srv/pipeline 10.11.0.0/16(ro,root_squash) fdff:dd...", lookup_key_id: 1104, created_at: "2016-06-15 14:33:48", updated_at: "2017-05-24 07:52:46", omit: false> with error unknown regexp options - "ppl". near line 1: " 10.11.0.0/16(ro,root_squash) fdff:dd77:2c2c:1100::/56(ro,root_squash)". Perhaps it is invalid?
/opt/theforeman/tfm/root/usr/share/gems/gems/ruby_parser-3.10.1/lib/ruby_lexer.rb:882:in `rb_compile_error'
/opt/theforeman/tfm/root/usr/share/gems/gems/ruby_parser-3.10.1/lib/ruby_lexer.rb:951:in `regx_options'
/opt/theforeman/tfm/root/usr/share/gems/gems/ruby_parser-3.10.1/lib/ruby_lexer.rb:1290:in `parse_string'
/opt/theforeman/tfm/root/usr/share/gems/gems/ruby_parser-3.10.1/lib/ruby_lexer.rb:1188:in `process_string'
/opt/theforeman/tfm/root/usr/share/gems/gems/ruby_parser-3.10.1/lib/ruby_lexer.rex.rb:72:in `next_token'
/opt/theforeman/tfm/root/usr/share/gems/gems/ruby_parser-3.10.1/lib/ruby_parser_extras.rb:967:in `next_token'
/opt/rh/rh-ruby24/root/usr/share/ruby/racc/parser.rb:259:in `_racc_do_parse_c'
/opt/rh/rh-ruby24/root/usr/share/ruby/racc/parser.rb:259:in `do_parse'
/opt/theforeman/tfm/root/usr/share/gems/gems/ruby_parser-3.10.1/lib/ruby_parser_extras.rb:1087:in `block in process'
/opt/rh/rh-ruby24/root/usr/share/ruby/timeout.rb:93:in `block in timeout'
/opt/rh/rh-ruby24/root/usr/share/ruby/timeout.rb:33:in `block in catch'
/opt/rh/rh-ruby24/root/usr/share/ruby/timeout.rb:33:in `catch'
/opt/rh/rh-ruby24/root/usr/share/ruby/timeout.rb:33:in `catch'
/opt/rh/rh-ruby24/root/usr/share/ruby/timeout.rb:108:in `timeout'
/opt/theforeman/tfm/root/usr/share/gems/gems/ruby_parser-3.10.1/lib/ruby_parser_extras.rb:1075:in `process'
/opt/theforeman/tfm/root/usr/share/gems/gems/ruby_parser-3.10.1/lib/ruby_parser.rb:31:in `block in process'
/opt/theforeman/tfm/root/usr/share/gems/gems/ruby_parser-3.10.1/lib/ruby_parser.rb:28:in `each'
/opt/theforeman/tfm/root/usr/share/gems/gems/ruby_parser-3.10.1/lib/ruby_parser.rb:28:in `process'
/opt/theforeman/tfm/root/usr/share/gems/gems/safemode-1.3.5/lib/safemode/parser.rb:18:in `parse'
/opt/theforeman/tfm/root/usr/share/gems/gems/safemode-1.3.5/lib/safemode/parser.rb:9:in `jail'
/opt/theforeman/tfm/root/usr/share/gems/gems/safemode-1.3.5/lib/safemode.rb:49:in `eval'
/usr/share/foreman/db/migrate/20180816134832_cast_lookup_key_values.rb:31:in `fix_value'
/usr/share/foreman/db/migrate/20180816134832_cast_lookup_key_values.rb:18:in `block in cast_key_and_values'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/relation/delegation.rb:39:in `each'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/relation/delegation.rb:39:in `each'
/usr/share/foreman/db/migrate/20180816134832_cast_lookup_key_values.rb:17:in `cast_key_and_values'
/usr/share/foreman/db/migrate/20180816134832_cast_lookup_key_values.rb:5:in `block in up'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/relation/batches.rb:63:in `block (2 levels) in find_each'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/relation/batches.rb:63:in `each'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/relation/batches.rb:63:in `block in find_each'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/relation/batches.rb:129:in `block in find_in_batches'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/relation/batches.rb:230:in `block in in_batches'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/relation/batches.rb:214:in `loop'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/relation/batches.rb:214:in `in_batches'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/relation/batches.rb:128:in `find_in_batches'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/relation/batches.rb:62:in `find_each'
/usr/share/foreman/db/migrate/20180816134832_cast_lookup_key_values.rb:4:in `up'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:795:in `exec_migration'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:776:in `block (2 levels) in migrate'
/opt/rh/rh-ruby24/root/usr/share/ruby/benchmark.rb:293:in `measure'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:775:in `block in migrate'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/connection_adapters/abstract/connection_pool.rb:408:in `with_connection'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:774:in `migrate'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:953:in `migrate'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:1230:in `block in execute_migration_in_transaction'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:1298:in `block in ddl_transaction'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/connection_adapters/abstract/database_statements.rb:235:in `block in transaction'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/connection_adapters/abstract/transaction.rb:194:in `block in within_new_transaction'
/opt/rh/rh-ruby24/root/usr/share/ruby/monitor.rb:214:in `mon_synchronize'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/connection_adapters/abstract/transaction.rb:191:in `within_new_transaction'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/connection_adapters/abstract/database_statements.rb:235:in `transaction'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/transactions.rb:210:in `transaction'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:1298:in `ddl_transaction'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:1229:in `execute_migration_in_transaction'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:1188:in `run_without_lock'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:1140:in `block in run'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:1317:in `with_advisory_lock'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:1140:in `run'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/migration.rb:1018:in `run'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/railties/databases.rake:99:in `block (3 levels) in <top (required)>'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/task.rb:250:in `block in execute'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/task.rb:250:in `each'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/task.rb:250:in `execute'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/task.rb:194:in `block in invoke_with_call_chain'
/opt/rh/rh-ruby24/root/usr/share/ruby/monitor.rb:214:in `mon_synchronize'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/task.rb:187:in `invoke_with_call_chain'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/task.rb:180:in `invoke'
/opt/theforeman/tfm-ror51/root/usr/share/gems/gems/activerecord-5.1.6/lib/active_record/railties/databases.rake:84:in `block (3 levels) in <top (required)>'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/task.rb:250:in `block in execute'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/task.rb:250:in `each'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/task.rb:250:in `execute'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/task.rb:194:in `block in invoke_with_call_chain'
/opt/rh/rh-ruby24/root/usr/share/ruby/monitor.rb:214:in `mon_synchronize'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/task.rb:187:in `invoke_with_call_chain'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/task.rb:180:in `invoke'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/application.rb:152:in `invoke_task'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/application.rb:108:in `block (2 levels) in top_level'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/application.rb:108:in `each'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/application.rb:108:in `block in top_level'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/application.rb:117:in `run_with_threads'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/application.rb:102:in `top_level'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/application.rb:80:in `block in run'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/application.rb:178:in `standard_exception_handling'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/lib/rake/application.rb:77:in `run'
/opt/rh/rh-ruby24/root/usr/share/gems/gems/rake-12.0.0/exe/rake:27:in `<top (required)>'
/opt/rh/rh-ruby24/root/usr/bin/rake:23:in `load'
/opt/rh/rh-ruby24/root/usr/bin/rake:23:in `<main>'
== 20180816134832 CastLookupKeyValues: migrated (0.2594s) =====================

#12 Updated by Tomer Brisker 3 months ago

The failed value indeed does not look like a valid yaml value, was it valid before the upgrade?

#13 Updated by Daniel Helgenberger 3 months ago

The failed value indeed does not look like a valid yaml value

However, it is valid yaml, just checked it with http://www.yamllint.com/. I just reverted to my 1.16.2 snapshot so I can confirm it is there before the migration.

#14 Updated by The Foreman Bot 3 months ago

  • Pull request https://github.com/theforeman/foreman/pull/5990 added

#15 Updated by The Foreman Bot 3 months ago

  • Pull request https://github.com/theforeman/foreman/pull/5995 added

#16 Updated by The Foreman Bot 3 months ago

  • Pull request https://github.com/theforeman/foreman/pull/5998 added

#17 Updated by Tomer Brisker 3 months ago

  • Fixed in Releases 1.17.4, 1.18.2 added

#18 Updated by Tomer Brisker 2 months ago

  • Related to Bug #23581: Upgrade to Foreman 1.17 converts YAML to JSON Hash added

Also available in: Atom PDF