Documentation for Federation Test Cases

[mdimjasevic]

Documents and provides federation test cases per https://wearezeta.atlassian.net/browse/SQCORE-1198.

@mythsunwind

[mythsunwind]

Thanks for marking me in the PR. I was able to include the test cases in the current state of this PR into the certification document.

Since all security functions are currently case sensitive please replace @SF.FEDERATION with @SF.Federation.

[pcapriotti]

LGTM modulo typos.

[fisx]

@mdimjasevic could you take another look at my changes and see if you think we've covered everything?

if ci is green this is good to merge IMO.

[mdimjasevic]

@fisx , sure, I will take a look!

[fisx]

ci passes locally.

[mdimjasevic]

@mdimjasevic could you take another look at my changes and see if you think we've covered everything?

if ci is green this is good to merge IMO.

It looks good to me. I cannot approve via the GitHub interface since I created the PR.

[fisx]

  test/integration/Test/Federator/InwardSpec.hs:89:11: 
  1) Federator.API.Inward shouldRejectMissmatchingOriginDomainInward
       uncaught exception: ErrorCall
       Assertions failed:
        1: 400 =/= 422
        2: Just "discovery-failure" =/= Just "invalid-domain"
       
       Response was:
       
       Response {responseStatus = Status {statusCode = 422, statusMessage = "Unprocessable Entity"}, responseVersion = HTTP/1.1, responseHeaders = [("Transfer-Encoding","chunked"),("Date","Fri, 17 Dec 2021 12:56:10 GMT"),("Server","Warp/3.3.17"),("Content-Type","application/json")], responseBody = Just "{\"code\":422,\"message\":\"srv record not found: _wire-server-federator._tcp.unknown-domain.com\",\"label\":\"invalid-domain\"}", responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose}
       CallStack (from HasCallStack):
         error, called at src/Bilge/Assert.hs:89:5 in bilge-0.22.0-KUsLZvxblgw5jARAGrpYx8:Bilge.Assert
         <!!, called at src/Bilge/Assert.hs:107:19 in bilge-0.22.0-KUsLZvxblgw5jARAGrpYx8:Bilge.Assert
         !!!, called at test/integration/Test/Federator/InwardSpec.hs:89:11 in main:Test.Federator.InwardSpec

looks like concourse is running things differently, leading to different errors in this test case. :-(

@mdimjasevic i suppose it's ok for you to post your typo corrections then...

[pcapriotti]

I forgot earlier, but the federation error documentation in swagger (https://github.com/wireapp/wire-server/blob/develop/services/brig/docs/swagger.md) should be updated to reflect any changes to status codes.

[mdimjasevic]

I forgot earlier, but the federation error documentation in swagger (https://github.com/wireapp/wire-server/blob/develop/services/brig/docs/swagger.md) should be updated to reflect any changes to status codes.

Thank you for pointing this out! It's been taken care of in a7123fac2e52d524ac0e920060348d60f53bb481.

[mdimjasevic]

looks like concourse is running things differently, leading to different errors in this test case. :-(

I updated the test case to use a non-existing domain, which should make it consistent with running the test locally.

@mdimjasevic i suppose it's ok for you to post your typo corrections then...

Done!

[fisx]

hum, why isn't this generated? (just wondering, please don't do anything about this in this PR :))

[mdimjasevic]

We decided to document the federation errors at the beginning of this Swagger document and then any endpoint that can throw a federation error can throw one of these. This is to avoid repeating the lengthy enumeration of federation errors over and over again at every endpoint that can throw it.

[fisx]

Suggested change

	runTestFederator env $ pure ()
	pure ()

[mdimjasevic]

I did not look thoroughly as to why, but just pure () doesn't type-check. I think that's because other it "blocks" in the spec use the runTestFederator env test values, which then GHC infers a type from, which then forces us to have the same type here, but I'd have to look deeper to confirm this.

[fisx]

ah, you're almost right: pure () can have the right type, it's an inference problem because both it and pure are polymorphic, and nothing determines any concrete type for m. you could write:

it "..." $ (pure () :: Assertion)

this would be much better if runTestFederator would set up expensive schaffolding (which is what I thought earlier), but since it's just Reader+IO, I would say just leave it as is.

[fisx]

ah, you're almost right: pure () can have the right type, it's an inference problem because both it and pure are polymorphic, and nothing determines any concrete type for m. you could write:

it "..." $ (pure () :: Assertion)

this would be much better if runTestFederator would set up expensive schaffolding (which is what I thought earlier), but since it's just Reader+IO, I would say just leave it as is.