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

Allow special characters in zabbixapi zabbix_pass & postgresql database_password #712

Closed
wants to merge 3 commits into from

Conversation

aclarkee
Copy link

This pull request fixes a couple of issues I came across regarding passwords with special characters not being handled correctly.

  1. Zabbixapi not accepting passwords with special characters (I'm not too familiar with ruby, but this fix works in my tests)
  2. postgresql echo exec not handling passwords with special characters correctly

@@ -49,7 +49,7 @@ def self.create_connection
zbx = ZabbixApi.connect(
url: "#{protocol}://#{api_config['default']['zabbix_url']}/api_jsonrpc.php",
user: api_config['default']['zabbix_user'],
Copy link
Member

Choose a reason for hiding this comment

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

should we quote all of the attribute here, just to be future safe? Can you please add a comment to that explains why the quotes were added? I'm afraid otherwise they will be removed in the future

Copy link
Member

Choose a reason for hiding this comment

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

Also, could we add test(s) for usage of special characters?

Copy link
Author

Choose a reason for hiding this comment

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

I'll make the same change for http_password - since it uses the same password. I don't think there'd be any benefit for making this change for user and http_user though?

@kenyon Completely new to specs, and not very familiar with ruby. I'll take a look at the existing specs and see if I can cobble something together. Would need a good review from someone more familiar with spec writing/ruby though.

Can I test specs locally, without having to push each change here for travis to test?

@@ -84,7 +84,7 @@
}

exec { 'update_pgpass':
command => "echo ${database_host}:5432:${database_name}:${database_user}:${database_password} >> /root/.pgpass",
command => "echo '${database_host}:5432:${database_name}:${database_user}:${database_password}' >> /root/.pgpass",
path => "/bin:/usr/bin:/usr/local/sbin:/usr/local/bin:${database_path}",
unless => "grep \"${database_host}:5432:${database_name}:${database_user}:${database_password}\" /root/.pgpass",
Copy link
Member

Choose a reason for hiding this comment

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

this grep should be adjusted as well to add the single quotes to existing files?

@bastelfreak bastelfreak added enhancement New feature or request needs-work not ready to merge just yet tests-fail labels Sep 12, 2020
@@ -31,7 +31,7 @@
end

it { is_expected.to compile.with_all_deps }
it { is_expected.to contain_exec('update_pgpass').with_command('echo node01.example.com:5432:zabbix-server:zabbix-server:zabbix-server >> /root/.pgpass') }
it { is_expected.to contain_exec('update_pgpass').with_command('echo 'node01.example.com:5432:zabbix-server:zabbix-server:zabbix-server' >> /root/.pgpass') }
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look right. Seems like there should be double quotation marks around the entire string, with single quotation marks within, like the lines below here.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. I'm completely new to writing specs - so that was a quick attempt. I'll try your suggestion later today

@vox-pupuli-tasks
Copy link

Dear @aclarkee, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@@ -49,7 +49,7 @@ def self.create_connection
zbx = ZabbixApi.connect(
url: "#{protocol}://#{api_config['default']['zabbix_url']}/api_jsonrpc.php",
user: api_config['default']['zabbix_user'],
password: api_config['default']['zabbix_pass'],
password: "#{api_config['default']['zabbix_pass']}",
Copy link
Member

Choose a reason for hiding this comment

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

I don't see why this would be needed. You're casting a string to another string.

@ekohl
Copy link
Member

ekohl commented Nov 7, 2023

In #904 I've removed the need for the pgpass file altogether.

@ekohl ekohl closed this Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request merge-conflicts needs-work not ready to merge just yet tests-fail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants