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

Handle xinetd config with only one entry #846

Merged
merged 1 commit into from
Aug 17, 2016

Conversation

chris-rock
Copy link
Contributor

This PR handles a bug reported by @kdshah1 via gitter:

is this the correct way to check xinetd services are disabled?

        describe xinetd_conf.services('telnet') do
         it{ should be_disabled }
       end

It fails with following message:

 ✖  services-04: System Must Disable telnet Service. (1 failed)
     expected `Xinetd config /etc/xinetd.conf with service == "telnet".disabled?` to return true, got false

whereas my telnet file under /etc/xinet.d reads this:

 # default: on
# description: The telnet server serves telnet sessions; it uses \
#       unencrypted username/password pairs for authentication.
service telnet
{
        disable = yes
        flags           = REUSE
        socket_type     = stream
        wait            = no
        user            = root
        server          = /usr/sbin/in.telnetd
        log_on_failure  += USERID
}

The problem was, that the xinetd resource was expecting multiple entries via includedir /etc/xinetd.d. This PR also adds another test to catch that case.

@chris-rock chris-rock added the Type: Bug Feature not working as expected label Jul 27, 2016
@chris-rock
Copy link
Contributor Author

should fix #845

# add the service identifier to its parameters
v.each { |service| service.params['service'] = name }
Copy link
Contributor

Choose a reason for hiding this comment

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

Minro style point. You might consider avoidng the type check and doing:

Array(v).each { |service| service.params['service'] = name }

Array(v) will coerce a scalar into an array but won't touch the arg if it is already an array:

Airb(main):001:0> Array(4)
=> [4]
irb(main):002:0> Array([4])
=> [4]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that makes it more save.

@stevendanna
Copy link
Contributor

Overall this looks good to me. I'm wondering if in the future the thing we actually want to store is a merged view of the config for a given service.

@chris-rock
Copy link
Contributor Author

Lets double-check if we should move this logic to the parser

@chris-rock chris-rock changed the title handle xinetd config with only one entry WIP: handle xinetd config with only one entry Aug 5, 2016
@chris-rock chris-rock modified the milestones: 0.30.0, 0.31.0 Aug 8, 2016
@chris-rock chris-rock force-pushed the chris-rock/fix-xinetd-single-entry branch from 62280d9 to 9f685a9 Compare August 16, 2016 10:23
@chris-rock chris-rock changed the title WIP: handle xinetd config with only one entry Handle xinetd config with only one entry Aug 16, 2016
@arlimus
Copy link
Contributor

arlimus commented Aug 16, 2016

I like the current version and the functionality in here 👍
Imho the discussion of splitting it up is a second iteration. Also is there a benefit of moving it to the parser? @stevendanna sounds like a great idea, let's track it as a separate idea

@chris-rock chris-rock merged commit 968e87b into master Aug 17, 2016
@chris-rock chris-rock deleted the chris-rock/fix-xinetd-single-entry branch August 17, 2016 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Feature not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants