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

DnsHost - Add server & type of query #13

Open
DexterPOSH opened this issue May 30, 2016 · 11 comments
Open

DnsHost - Add server & type of query #13

DexterPOSH opened this issue May 30, 2016 · 11 comments

Comments

@DexterPOSH
Copy link

DexterPOSH commented May 30, 2016

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}
    }
}
@cdhunt
Copy link
Contributor

cdhunt commented May 30, 2016

That's a great idea.

@cdhunt cdhunt added this to the Next milestone May 30, 2016
@DexterPOSH
Copy link
Author

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

@cdhunt
Copy link
Contributor

cdhunt commented May 30, 2016

@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?

@DexterPOSH
Copy link
Author

DexterPOSH commented May 30, 2016

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
Copy link
Author

DexterPOSH commented May 30, 2016

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 ?

@cdhunt
Copy link
Contributor

cdhunt commented May 30, 2016

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 cdhunt added the RFC label May 30, 2016
@DexterPOSH
Copy link
Author

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 👍
I will re-work on creating separate DNSARecord and DNSMXRecord functions now.

@asipe
Copy link

asipe commented Aug 2, 2016

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?

@DexterPOSH
Copy link
Author

DexterPOSH commented Aug 2, 2016

@asipe I ended up doing few changes in PoshSpec (ugly ones) 😝
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).

@cdhunt
Copy link
Contributor

cdhunt commented Aug 2, 2016

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

@DexterPOSH
Copy link
Author

DexterPOSH commented Jan 18, 2017

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants