From d689704739abac1711403b3379537e80de212d91 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Mon, 10 Jun 2024 13:20:18 +0900 Subject: [PATCH] Remove support for json format policy files Json format was deprecated multiple cycles ago in favor of the new yaml format. Change-Id: Ie43d9cc2441bd864e13ec792e1f6ce76dda354cf --- manifests/policy.pp | 7 +- manifests/policy/base.pp | 66 +++++-------------- manifests/policy/default.pp | 29 ++------ .../remove-policy-json-8902dc11c5576e73.yaml | 5 ++ spec/defines/openstacklib_policy_base_spec.rb | 64 ++---------------- .../openstacklib_policy_default_spec.rb | 31 +-------- spec/defines/openstacklib_policy_spec.rb | 41 ++---------- 7 files changed, 43 insertions(+), 200 deletions(-) create mode 100644 releasenotes/notes/remove-policy-json-8902dc11c5576e73.yaml diff --git a/manifests/policy.pp b/manifests/policy.pp index 8e469134..3e805eda 100644 --- a/manifests/policy.pp +++ b/manifests/policy.pp @@ -43,9 +43,8 @@ # Defaults to undef # # [*file_format*] -# (Optional) Format for file contents. Valid values -# are 'json' or 'yaml'. -# Defaults to 'json'. +# (Optional) Format for file contents. Valid value is 'yaml'. +# Defaults to 'yaml'. # # [*purge_config*] # (Optional) Whether to set only the specified policy rules in the policy @@ -58,7 +57,7 @@ define openstacklib::policy ( $file_mode = '0640', $file_user = undef, $file_group = undef, - Enum['json', 'yaml'] $file_format = 'json', + Enum['yaml'] $file_format = 'yaml', Boolean $purge_config = false, ) { diff --git a/manifests/policy/base.pp b/manifests/policy/base.pp index efdbeddc..34a74210 100644 --- a/manifests/policy/base.pp +++ b/manifests/policy/base.pp @@ -1,11 +1,11 @@ # == Definition: openstacklib::policy::base # -# This resource configures the policy.json file for an OpenStack service +# This resource configures the policy file for an OpenStack service # # == Parameters: # # [*file_path*] -# (required) Path to the policy.json file +# (required) Path to the policy file # # [*key*] # (optional) The key to replace the value for @@ -28,9 +28,8 @@ # Defaults to undef # # [*file_format*] -# (optional) Format for file contents. Valid values -# are 'json' or 'yaml'. -# Defaults to 'json'. +# (optional) Format for file contents. Valid value is 'yaml' +# Defaults to 'yaml'. # # [*purge_config*] # (optional) Whether to set only the specified policy rules in the policy @@ -44,7 +43,7 @@ define openstacklib::policy::base ( $file_mode = '0640', $file_user = undef, $file_group = undef, - Enum['json', 'yaml'] $file_format = 'json', + Enum['yaml'] $file_format = 'yaml', Boolean $purge_config = false, ) { @@ -57,50 +56,17 @@ define openstacklib::policy::base ( purge_config => $purge_config }) - case $file_format { - 'json': { - warning('Json format is deprecated and will be removed in a future release') + # NOTE(tkajianm): Currently we use single quotes('') to quote the whole + # value, thus a single quote in value should be escaped + # by another single quote (which results in '') + # NOTE(tkajinam): Replace '' by ' first in case ' is already escaped + $value_real = regsubst(regsubst($value, '\'\'', '\'', 'G'), '\'', '\'\'', 'G') - # Add entry if it doesn't exists - augeas { "${file_path}-${key}-add": - lens => 'Json.lns', - incl => $file_path, - changes => [ - "set dict/entry[last()+1] \"${key}\"", - "set dict/entry[last()]/string \"${value}\"", - ], - onlyif => "match dict/entry[*][.=\"${key}\"] size == 0", - } - - # Requires that the entry is added before this call or it will fail. - augeas { "${file_path}-${key}" : - lens => 'Json.lns', - incl => $file_path, - changes => "set dict/entry[*][.=\"${key}\"]/string \"${value}\"", - } - - Openstacklib::Policy::Default<| title == $file_path |> - -> Augeas<| title == "${file_path}-${key}-add" |> - ~> Augeas<| title == "${file_path}-${key}" |> - } - 'yaml': { - # NOTE(tkajianm): Currently we use single quotes('') to quote the whole - # value, thus a single quote in value should be escaped - # by another single quote (which results in '') - # NOTE(tkajinam): Replace '' by ' first in case ' is already escaped - $value_real = regsubst(regsubst($value, '\'\'', '\'', 'G'), '\'', '\'\'', 'G') - - file_line { "${file_path}-${key}" : - path => $file_path, - line => "'${key}': '${value_real}'", - match => "^['\"]?${key}(?!:)['\"]?\\s*:.+" - } - Openstacklib::Policy::Default<| title == $file_path |> - -> File_line<| title == "${file_path}-${key}" |> - } - default: { - fail("${file_format} is an unsupported policy file format. Choose 'json' or 'yaml'.") - } + file_line { "${file_path}-${key}" : + path => $file_path, + line => "'${key}': '${value_real}'", + match => "^['\"]?${key}(?!:)['\"]?\\s*:.+" } - + Openstacklib::Policy::Default<| title == $file_path |> + -> File_line<| title == "${file_path}-${key}" |> } diff --git a/manifests/policy/default.pp b/manifests/policy/default.pp index cd548c85..0c4353e7 100644 --- a/manifests/policy/default.pp +++ b/manifests/policy/default.pp @@ -5,7 +5,7 @@ # == Parameters: # # [*file_path*] -# (Optional) Path to the policy.json file +# (Optional) Path to the policy file # Defaults to $name # # [*file_mode*] @@ -21,9 +21,8 @@ # Defaults to undef # # [*file_format*] -# (Optional) Format for file contents. Valid values -# are 'json' or 'yaml'. -# Defaults to 'json'. +# (Optional) Format for file contents. Valid value is 'yaml'. +# Defaults to 'yaml'. # # [*purge_config*] # (Optional) Whether to set only the specified policy rules in the policy @@ -35,33 +34,15 @@ define openstacklib::policy::default ( $file_mode = '0640', $file_user = undef, $file_group = undef, - Enum['json', 'yaml'] $file_format = 'json', + Enum['yaml'] $file_format = 'yaml', Boolean $purge_config = false, ) { - case $file_format { - 'json': { - warning('Json format is deprecated and will be removed in a future release') - $content = '{}' - } - 'yaml': { - if stdlib::extname($file_path) == '.json' { - # NOTE(tkajinam): It is likely that user is not aware of migration from - # policy.json to policy.yaml - fail("file_path: ${file_path} should be a yaml file instead of a json file") - } - $content = '' - } - default: { - fail("${file_format} is an unsupported policy file format. Choose 'json' or 'yaml'.") - } - } - ensure_resource('file', $file_path, { mode => $file_mode, owner => $file_user, group => $file_group, replace => $purge_config, - content => $content + content => '' }) } diff --git a/releasenotes/notes/remove-policy-json-8902dc11c5576e73.yaml b/releasenotes/notes/remove-policy-json-8902dc11c5576e73.yaml new file mode 100644 index 00000000..d704c793 --- /dev/null +++ b/releasenotes/notes/remove-policy-json-8902dc11c5576e73.yaml @@ -0,0 +1,5 @@ +--- +upgrade: + - | + Support for json format policy files has been removed. Now yaml is the only + supported format. diff --git a/spec/defines/openstacklib_policy_base_spec.rb b/spec/defines/openstacklib_policy_base_spec.rb index 40ccf126..e97bfba2 100644 --- a/spec/defines/openstacklib_policy_base_spec.rb +++ b/spec/defines/openstacklib_policy_base_spec.rb @@ -6,52 +6,14 @@ describe 'openstacklib::policy::base' do 'context_is_admin or owner' end - context 'with policy.json' do - let :params do - { - :file_path => '/etc/nova/policy.json', - :value => 'foo:bar', - :file_mode => '0644', - :file_user => 'foo', - :file_group => 'bar', - :file_format => 'json', - } - end - - it { should contain_openstacklib__policy__default('/etc/nova/policy.json').with( - :file_mode => '0644', - :file_user => 'foo', - :file_group => 'bar', - :file_format => 'json', - :purge_config => false, - )} - - it { should contain_augeas('/etc/nova/policy.json-context_is_admin or owner').with( - :lens => 'Json.lns', - :incl => '/etc/nova/policy.json', - :changes => 'set dict/entry[*][.="context_is_admin or owner"]/string "foo:bar"', - )} - - it { should contain_augeas('/etc/nova/policy.json-context_is_admin or owner-add').with( - :lens => 'Json.lns', - :incl => '/etc/nova/policy.json', - :changes => [ - 'set dict/entry[last()+1] "context_is_admin or owner"', - 'set dict/entry[last()]/string "foo:bar"' - ], - :onlyif => 'match dict/entry[*][.="context_is_admin or owner"] size == 0' - )} - end - context 'with policy.yaml' do let :params do { - :file_path => '/etc/nova/policy.yaml', - :value => 'foo:bar', - :file_mode => '0644', - :file_user => 'foo', - :file_group => 'bar', - :file_format => 'yaml', + :file_path => '/etc/nova/policy.yaml', + :value => 'foo:bar', + :file_mode => '0644', + :file_user => 'foo', + :file_group => 'bar', } end @@ -106,7 +68,6 @@ describe 'openstacklib::policy::base' do :file_mode => '0644', :file_user => 'foo', :file_group => 'bar', - :file_format => 'yaml', :purge_config => true, } end @@ -120,21 +81,6 @@ describe 'openstacklib::policy::base' do )} end - context 'with json file_path and yaml file format' do - let :params do - { - :file_path => '/etc/nova/policy.json', - :value => 'foo:bar', - :file_mode => '0644', - :file_user => 'foo', - :file_group => 'bar', - :file_format => 'yaml', - } - end - - it { should raise_error(Puppet::Error) } - end - context 'with key overridden' do let :params do { diff --git a/spec/defines/openstacklib_policy_default_spec.rb b/spec/defines/openstacklib_policy_default_spec.rb index 1ceee2b9..e042f576 100644 --- a/spec/defines/openstacklib_policy_default_spec.rb +++ b/spec/defines/openstacklib_policy_default_spec.rb @@ -2,29 +2,6 @@ require 'spec_helper' describe 'openstacklib::policy::default' do shared_examples 'openstacklib::policy::default' do - context 'with policy.json' do - let :title do - '/etc/nova/policy.json' - end - - let :params do - { - :file_mode => '0644', - :file_user => 'foo', - :file_group => 'bar', - :file_format => 'json', - } - end - - it { should contain_file('/etc/nova/policy.json').with( - :mode => '0644', - :owner => 'foo', - :group => 'bar', - :content => '{}', - :replace => false - )} - end - context 'with policy.yaml' do let :title do '/etc/nova/policy.yaml' @@ -32,10 +9,9 @@ describe 'openstacklib::policy::default' do let :params do { - :file_mode => '0644', - :file_user => 'foo', - :file_group => 'bar', - :file_format => 'yaml', + :file_mode => '0644', + :file_user => 'foo', + :file_group => 'bar', } end @@ -58,7 +34,6 @@ describe 'openstacklib::policy::default' do :file_mode => '0644', :file_user => 'foo', :file_group => 'bar', - :file_format => 'yaml', :purge_config => true, } end diff --git a/spec/defines/openstacklib_policy_spec.rb b/spec/defines/openstacklib_policy_spec.rb index ff1b7813..45f3b090 100644 --- a/spec/defines/openstacklib_policy_spec.rb +++ b/spec/defines/openstacklib_policy_spec.rb @@ -3,49 +3,21 @@ require 'spec_helper' describe 'openstacklib::policy' do shared_examples 'openstacklib::policy' do context 'with basic configuration' do - let :title do - '/etc/nova/policy.json' - end - - let :params do - { - :policies => { - 'foo' => { - 'key' => 'context_is_admin', - 'value' => 'foo:bar' - } - }, - :file_mode => '0644', - :file_user => 'foo', - :file_group => 'baa', - :file_format => 'json', - } - end - - it { should contain_openstacklib__policy__base('foo').with( - :file_path => '/etc/nova/policy.json', - :key => 'context_is_admin', - :value => 'foo:bar' - )} - end - - context 'with yaml configuration' do let :title do '/etc/nova/policy.yaml' end let :params do { - :policies => { + :policies => { 'foo' => { - 'key' => 'context_is_admin', - 'value' => 'foo:bar' + 'key' => 'context_is_admin', + 'value' => 'foo:bar' } }, - :file_mode => '0644', - :file_user => 'foo', - :file_group => 'baa', - :file_format => 'yaml', + :file_mode => '0644', + :file_user => 'foo', + :file_group => 'baa', } end @@ -66,7 +38,6 @@ describe 'openstacklib::policy' do :file_mode => '0644', :file_user => 'foo', :file_group => 'baa', - :file_format => 'yaml', :purge_config => true, } end