poshspec icon indicating copy to clipboard operation
poshspec copied to clipboard

DnsHost - Add server & type of query

Open DexterPOSH opened this issue 8 years ago • 11 comments

Does the above make sense to be added to the DnsHost implementation ?

I am working on project, where we have multiple DNS servers set on the client nodes and want to validate that the name resolution works using each of the DNS server, also at the same time be able to make different type of DNS queries to the DNS server.

Current implementation of the DnsHost does not let you specify a DNS server or the type of DNS query to for a validation test.

For Example - a machine can have multiple DNS servers, so this test should allow specifying a specific DNS server for the name resolution along with the type of DNS query to run for the test e.g MX or an A record query etc.

Describe 'multiple DNS servers' {

    Context 'Test for the DNS server1' {
        DnsHost cloud.local 10.116.2.250  {Should Not BeNullOrEmpty} # query the domain FQDN
        DnsHost mail.cloud.local 10.116.2.250 -Type MX {Should Not BeNullOrEmpty} # test the MX record
    }

    Context 'Test for the DNS server2' {
        DnsHost cloud.local 10.116.2.251 {Should Not BeNullOrEmpty}
        DnsHost mail.cloud.local 10.116.2.251 -Type MX {Should Not BeNullOrEmpty}
    }
}

DexterPOSH avatar May 30 '16 08:05 DexterPOSH

That's a great idea.

cdhunt avatar May 30 '16 13:05 cdhunt

@cdhunt I have that already working in my local repo (minor changes). Should I create a PR ?

DexterPOSH avatar May 30 '16 13:05 DexterPOSH

@DexterPOSH I really prefer to not implement test specific parameters like Type. I think I would prefer to just create separate functions like DnsARecord and DnsMxRecord. By using just the Property and Qualifier parameters, the test names remain consistent and meaningful without discarding the work done by Get-PoshspecParam. Does that make sense?

cdhunt avatar May 30 '16 14:05 cdhunt

But this would bring a lot of extra work. For Example there are a lot of DNS query types, I have something like below in the validateset for the Type

# Type of the Name query
        [Parameter()]
        [ValidateSet('A_AAAA','A','AAAA','NS','MX','MD','MF','CNAME','SOA',
        'MB','MG','MR','NULL','WKS','PTR','HINFO','MINFO','TXT','RP','AFSDB',
        'X25','ISDN','RT','SRV','DNAME','OPT','DS','RRSIG','NSEC','DNSKEY',
        'DHCID','NSEC3','NSEC3PARAM','ANY','ALL')]
        [String]$Type

So one would need to create those many separate DNSRecord functions, hope you get my point.

DexterPOSH avatar May 30 '16 14:05 DexterPOSH

I understand trying to have consistent parameters for all the Functions being added.

Since these arguments are actually being passed to the underlying cmdlet, Can't there be a generic parameter like ArgumentList (Accepts hashtable) here ? Purpose of it would be to simply append the key-value pairs to the underlying cmdlet expression , Make sense ?

DexterPOSH avatar May 30 '16 14:05 DexterPOSH

Each Dns record will likely have different testing logic. You don't necessarily need to implement a function for all types. Just work on what helps you. However, as an example, testing a CNAME you might want to test the NameHost property or the IP4Address of the resolved A record. By sticking all of that logic into one function, you will end up with a bunch of tests with very similar Names and it will be more difficult to discern from the results what was tested.

You can also create you own hash with Expression and Name keys to pass to Invoke-PoshspecExpression or you can manually construct It statement without using the helpers.

How you are designing the function doesn't fit into the pattern of Poshspec. It's perfectly valid from a standalone function standpoint, but I would like each function in Poshspec to be consistent and simple so users do not have to look up detailed Help for every type of test they want to use.

I would like to open it up for discussion with others If you want to submit the PR as you have it.

cdhunt avatar May 30 '16 14:05 cdhunt

hmmm....having read your response, I understand that adding test specific params would open a can of worms. For the time being writing the IT blocks for my own specific use case makes more sense :+1: I will re-work on creating separate DNSARecord and DNSMXRecord functions now.

DexterPOSH avatar May 31 '16 03:05 DexterPOSH

I was faced with a similar problem when I read this thread.

My particular issue is that I need to get some additional headers passed to the Http function. I was going to implement this as an additional optional parameter. After reading this thread it doesn't sound like that is the way to go. Any thoughts on how something like this might be better implemented?

asipe avatar Aug 02 '16 14:08 asipe

@asipe I ended up doing few changes in PoshSpec (ugly ones) :stuck_out_tongue_closed_eyes: Which lets you do something like this

DNSHost @{Name='cloud.local';Server='10.116.2.250'} { Should NOT Be $null }

This simply splats the target hash to the Resolve-DNSName but it works for me at the moment. So you can pass any additional parameters to the underlying function, but this meant I had to update every public function for that (remember I said ugly).

DexterPOSH avatar Aug 02 '16 14:08 DexterPOSH

Taking a hashtable of additional Cmdlet parameters is a pretty good generic solution. That aligns with how MS does the Requires -Module syntax.

cdhunt avatar Aug 02 '16 14:08 cdhunt

@cdhunt I have sort of incomplete implementation of the above working in my branch. To summarize had to modify the Target parameter to take an Object type and then make decisions based on whether it is a string or hashtable. If a hashtable is passed as a target, then it is splatted to the underlying cmdlet in the expression.

For Example

Describe 'CIMObjects' {
    CimObject Win32_OperatingSystem SystemDirectory { Should Be C:\WINDOWS\system32 }
    CimObject @{ClassName='Win32_service';Filter="Name='WinRm'"} Status  {Should be 'Ok'} 
}

Describe 'DNSHost' {
    DnsHost Google.com {Should not be $null}
    DnsHost @{name='Google.com';Type='A'} {Should not be $null}
}

Let me know your thoughts on the implementation. Branch is here P.S. - I have only added this ability to few of the underlying Public types at the moment.

DexterPOSH avatar Jan 18 '17 10:01 DexterPOSH