search

Kuadrant / testsuite

4 stars 14 forks source link

Add test for invalid credentials in dns provider #520

Closed averevki closed 1 month ago

averevki commented 2 months ago

Closes #519

trepel commented 2 months ago

@averevki this test looks good but I failed to make it pass on kua-stage-single today. First the DNSPolicy message was 'DNSPolicy has encountered some issues: policy is not enforced on any DNSRecord: no routes attached for listeners'

So I made sure that HTTPRoute is also created as part of the test. Then the record condition looked like:

  recordConditions:
    '*.5qdpemu.aws.kua.app-services-dev.net':
      - lastTransitionTime: '2024-08-28T13:36:02Z'
        message: "Unable to find suitable zone in provider: failed to list hosted zones, InvalidClientTokenId: The security token included in the request is invalid.\n\tstatus code: 403, request id: 548c40ce-29aa-4b24-ac3b-77a495590b66"
        observedGeneration: 1
        reason: DNSProviderError
        status: 'False'
        type: Ready

So I think that DNSProviderError should be used as Reason.

However, I was not able to verify this because I was unable to get to that point again for some reason. No idea why. Both Gateway and DNSPolicy were updated constantly by operators and DNSPolicy was saying that: 'DNSPolicy has encountered some issues: reconcile DNSRecords error error reconciling dns records for gateway gw-trepel--f41m: Operation cannot be fulfilled on dnsrecords.kuadrant.io "gw-trepel--f41m-api": the object has been modified; please apply your changes to the latest version and try again'

trepel commented 2 months ago

Also, when running the test I noticed that the GW reporting errors related to TLS setup - TLS policy is not committed at all. Not sure if that makes a difference, I fixed this too but it did not help me make this test pass.

trepel commented 2 months ago

However, I was not able to verify this because I was unable to get to that point again for some reason. No idea why. Both Gateway and DNSPolicy were updated constantly by operators and DNSPolicy was saying that: 'DNSPolicy has encountered some issues: reconcile DNSRecords error error reconciling dns records for gateway gw-trepel--f41m: Operation cannot be fulfilled on dnsrecords.kuadrant.io "gw-trepel--f41m-api": the object has been modified; please apply your changes to the latest version and try again'

I was not able to reproduce this today. Strange. But my other two comments seem to be valid: 1) HTTPRoute needs to be created 2) DNSProviderError should be set as 'Reason' parameter to has_record_condition

trepel commented 2 months ago

Slightly unrelated but can you think of an easy way to implement so that there is clear what the DNSPolicy status was and what the expected status is when the last assertion fails? I can only see what is expected but not really the actual status.

averevki commented 2 months ago

Slightly unrelated but can you think of an easy way to implement so that there is clear what the DNSPolicy status was and what the expected status is when the last assertion fails? I can only see what is expected but not really the actual status.

Do you mean to check for successful enforcement first and then provoke the error status? Or just add comments to the assertions?

trepel commented 2 months ago

Slightly unrelated but can you think of an easy way to implement so that there is clear what the DNSPolicy status was and what the expected status is when the last assertion fails? I can only see what is expected but not really the actual status.

Do you mean to check for successful enforcement first and then provoke the error status? Or just add comments to the assertions?

What I meant was that the test failed for me on that last assertion and it was not clear from the test log what the actual dnspolicy status was. It would be nice if we could see both what the expected status of dns policy and actual status of dns policy were.

averevki commented 2 months ago

@trepel assert message should be clearer now. Although I see we have this problem in more places where it can be improved later

averevki commented 1 month ago

LGTM now. Not sure how stable this test will be given the constant DNSPolicy CR status updates but there is no way of knowing beforehand. It passed for me, also the assertion output in case of failure is much more informative. I think this is good to go, we can skip it if it fails in nightlies too frequently.

Constant status updates should be fixed now :+1: (https://github.com/Kuadrant/dns-operator/issues/218)

averevki commented 1 month ago

rebase^