Update oracledb_session.rb

[pacopal]

Description

Running inspec from the database servers and using oracledb_session will fail, when the database server version is 12.1 or 11g. This is because the sqlplus version installed in the database servers will match with the database versions.

oracledb_session is using the following format options:
format_options = "SET MARKUP CSV ON\nSET PAGESIZE 32000\nSET FEEDBACK OFF"

Unfortunately "SET MARKUP CSV ON" was introduced in sqlplus version 12.2.0.1. So prior versions will miss it.
Reference:
https://docs.oracle.com/en/database/oracle/oracle-database/12.2/sqprn/index.html#SQPRN-GUID-98CEF1F8-FB5A-495F-8771-693546655613

The workaround provided by this PR is to replace the format options with a more generic set:
format_options = "SET PAGESIZE 32000\nSET FEEDBACK OFF\nSET UNDERLINE OFF"

Related Issue

oracledb_session doesnt work with sqlplus 12.1 or lower.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New content (non-breaking change)
• Breaking change (a content change which would break existing functionality or processes)

Checklist:

• I have read the CONTRIBUTING document.

[netlify]

Deploy preview for chef-inspec failed.

Built with commit 546501e

https://app.netlify.com/sites/chef-inspec/deploys/5fbbbe92cad75300076f7c7c

[chef-expeditor]

Hello pacopal! Thanks for the pull request!

Here is what will happen next:

1. Your PR will be reviewed by the maintainers.
2. Possible Outcomes
   a. If everything looks good, one of them will approve it, and your PR will be merged.
   b. The maintainer may request follow-on work (e.g. code fix, linting, etc). We would encourage you to address this work in 2-3 business days to keep the conversation going and to get your contribution in sooner.
   c. Cases exist where a PR is neither aligned to Chef InSpec's product roadmap, or something the team can own or maintain long-term. In these cases, the maintainer will provide justification and close out the PR.

Thank you for contributing!

[Schwad]

Hi @pacopal - that seems like reasonable enough of a PR to me.

It does look like you have some spec failures with your change that might be worth looking at : https://buildkite.com/chef-oss/inspec-inspec-master-verify/builds/2268

[Schwad]

Hello @pacopal ,

Any thoughts what might be causing these issues? https://buildkite.com/chef-oss/inspec-inspec-master-verify/builds/2268#32f87fdf-1936-4e04-aec5-356941ecbb57

1) Error:
--
  | Inspec::Resources::OracledbSession#test_0001_sqlplus Linux:
  | RuntimeError: "/bin/sqlplus -S \"USER\"/\"password\"@localhost:1527/ORCL <<'EOC'\nSET PAGESIZE 32000\nSET FEEDBACK OFF\nSET UNDERLINE OFF\nSELECT NAME AS VALUE FROM v$database;\nEXIT\nEOC"
  | /workdir/test/unit/resources/oracledb_session_test.rb:13:in `block (3 levels) in <top (required)>'
  | /workdir/lib/inspec/resources/oracledb_session.rb:55:in `query'
  | /workdir/test/unit/resources/oracledb_session_test.rb:18:in `block (2 levels) in <top (required)>'
  |  
  | 2) Error:
  | Inspec::Resources::OracledbSession#test_0002_sqlplus Windows:
  | RuntimeError: "@'\nSET PAGESIZE 32000\nSET FEEDBACK OFF\nSET UNDERLINE OFF\nSELECT NAME AS VALUE FROM v$database;\nEXIT\n'@ \| C:/sqlplus.exe -S \"USER\"/\"password\"@localhost:1527/ORCL"
  | /workdir/test/unit/resources/oracledb_session_test.rb:30:in `block (3 levels) in <top (required)>'
  | /workdir/lib/inspec/resources/oracledb_session.rb:55:in `query'
  | /workdir/test/unit/resources/oracledb_session_test.rb:35:in `block (2 levels) in <top (required)>'
  |  
  | 2821 runs, 8502 assertions, 0 failures, 2 errors, 10 skips

[pacopal]

Hi,
apparently those test units are comparing the test command line with the hard-coded values from the original code.

Extract:

describe "Inspec::Resources::OracledbSession" do
  it "sqlplus Linux" do
    resource = quick_resource(:oracledb_session, :linux, user: "USER", password: "password", host: "localhost", service: "ORCL", port: 1527, sqlplus_bin: "/bin/sqlplus") do |cmd|
      cmd.strip!
      case cmd
      when "/bin/sqlplus -S \"USER\"/\"password\"@localhost:1527/ORCL <<'EOC'\nSET MARKUP CSV ON\nSET PAGESIZE 32000\nSET FEEDBACK OFF\nSELECT NAME AS VALUE FROM v$database;\nEXIT\nEOC" then
        stdout_file "test/fixtures/cmd/oracle-result"
      else
        raise cmd.inspect
      end
    end

That when comparison will of course be false as we are removing in this Pull Request the "SET MARKUP CSV ON", etc.
Heading the "raise cmd.inspect" command.

I fail to understand why such case condition would be needed in the Resource... Maybe to raise an exception if a failed command string has been generated?
But if it is required, should be changed with the new values.

Should I change it?
Am I allowed to modify Inspec::Resources::OracledbSession accordingly ?

[pacopal]

@Schwad Hi, I have pushed the code change also for the Test Units. I hope this helps...

[Schwad]

Hi @pacopal - sorry for the delay! 😅 Things are looking good, I approved the rest of the tests. If this passes, would you be able to squash your commits? Thanks!

[aaronlippold]

:bump: does this just need a fresh rebase - the update to the resource would be useful for my teams as well.

[Schwad]

:bump: does this just need a fresh rebase - the update to the resource would be useful for my teams as well.

@aaronlippold - a fresh rebase would be okay but this is currently only blocked as we're awaiting the commits being squashed

[aaronlippold]

We are also seeing some odd issues with RDS instances as well ... I will either create a new issue or add to this one

[pacopal]

Hi.
To be honest, I am a bit lost on what is expected on my end. :) Is there action item pending for me?
The netlify/chef-inspec/deploy-preview test was not successful....
Do I assume properly that we need to fix that before merging?

[Schwad]

@pacopal yes - the one outstanding item for you is to squash this from 6 commits into 1 commit.

https://medium.com/@slamflipstrom/a-beginners-guide-to-squashing-commits-with-git-rebase-8185cf6e62ec

If you would like more guidance around how git squashing works just let me know and I'll help you out

[pacopal]

@Schwad
Done... but apparently I got new errors.
Did I screwed up something while doing the squash?
Or... Are those new tests added recently?

[clintoncwolfe]

The test errors - due to rspec version changing the text output of "falsey" - have nothing to do with your PR. This can merge.