select relevant third-party tests to run their tests with garble on CI #240
Milestone
Comments
|
Also, this is blocked by #241. |
|
I forgot to say what is perhaps obvious; We already kind of cover this in |
|
Here's a complication: by design, some tests will fail when obfuscated: Unfortunately there isn't an opposite to Another option might be to use |
|
As a first goal, this should check that the third party packages build. #310 is a good example of when we can't even build due to garble bugs. |
mvdan
added a commit
to mvdan/garble-fork
that referenced
this issue
Apr 25, 2021
Packages P1 and P2 can define identical struct types T1 and T2, and one can convert from type T1 to T2 or vice versa. The spec defines two identical struct types as: Two struct types are identical if they have the same sequence of fields, and if corresponding fields have the same names, and identical types, and identical tags. Non-exported field names from different packages are always different. Unfortunately, garble broke this: since we obfuscated field names differently depending on the package, cross-package conversions like the case above would result in typechecking errors. To fix this, implement Joe Tsai's idea: hash struct field names with the string representation of the entire struct. This way, identical struct types will have their field names obfuscated in the same way in all packages across a build. Note that we had to refactor "reverse" a bit to start using transformer, since now it needs to keep track of struct types as well. This failure was affecting the build of google.golang.org/protobuf, since it makes regular use of cross-package struct conversions. Note that the protobuf module still fails to build, but for other reasons. The package that used to fail now succeeds, so the build gets a bit further than before. burrowers#240 tracks adding relevant third-party Go modules to CI, so we'll track the other remaining failures there. Fixes burrowers#310.
lu4p
pushed a commit
that referenced
this issue
Apr 25, 2021
Packages P1 and P2 can define identical struct types T1 and T2, and one can convert from type T1 to T2 or vice versa. The spec defines two identical struct types as: Two struct types are identical if they have the same sequence of fields, and if corresponding fields have the same names, and identical types, and identical tags. Non-exported field names from different packages are always different. Unfortunately, garble broke this: since we obfuscated field names differently depending on the package, cross-package conversions like the case above would result in typechecking errors. To fix this, implement Joe Tsai's idea: hash struct field names with the string representation of the entire struct. This way, identical struct types will have their field names obfuscated in the same way in all packages across a build. Note that we had to refactor "reverse" a bit to start using transformer, since now it needs to keep track of struct types as well. This failure was affecting the build of google.golang.org/protobuf, since it makes regular use of cross-package struct conversions. Note that the protobuf module still fails to build, but for other reasons. The package that used to fail now succeeds, so the build gets a bit further than before. #240 tracks adding relevant third-party Go modules to CI, so we'll track the other remaining failures there. Fixes #310.
|
Another important point is that, for these relevant third-party codebases, we should try building them with a variety of GOOS/GOARCH targets that CI doesn't directly cover. For example:
For instance, the first of those three would have caught #417 just by using |
|
From #410, it seems like Wireguard (the Go implementation) should also be in the list of relevant third-party tests. |
mvdan
added a commit
to mvdan/garble-fork
that referenced
this issue
May 13, 2022
Our tests should already be pretty extensive, and any bug fixes should result in more regression test cases, but testing against a few diverse and popular third party modules will help prevent unintended regressions while developing garble. The list is short for now. More can be added later. This adds protobuf and wireguard from the original issue, but not cobra and logrus, as they aren't particularly complex nor add significant variety on top of protobuf and wireguard. While here, we remove the job that only runs crlf-test.sh, as we don't really need a separate job for a tiny script. Fixes burrowers#240.
mvdan
added a commit
to mvdan/garble-fork
that referenced
this issue
May 13, 2022
Our tests should already be pretty extensive, and any bug fixes should result in more regression test cases, but testing against a few diverse and popular third party modules will help prevent unintended regressions while developing garble. The list is short for now. More can be added later. This adds protobuf and wireguard from the original issue, but not cobra and logrus, as they aren't particularly complex nor add significant variety on top of protobuf and wireguard. While here, we remove the job that only runs crlf-test.sh, as we don't really need a separate job for a tiny script. Fixes burrowers#240.
mvdan
added a commit
that referenced
this issue
May 14, 2022
Our tests should already be pretty extensive, and any bug fixes should result in more regression test cases, but testing against a few diverse and popular third party modules will help prevent unintended regressions while developing garble. The list is short for now. More can be added later. This adds protobuf and wireguard from the original issue, but not cobra and logrus, as they aren't particularly complex nor add significant variety on top of protobuf and wireguard. While here, we remove the job that only runs crlf-test.sh, as we don't really need a separate job for a tiny script. Fixes #240.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
For example, #190 shows us that programs using basic protobuf-generated code panic at init time. We could (and probably should) try to minimize the failure into a test case in garble, but we should also ensure that
garble test ./...in a recent version of protobuf succeeds.I propose this short list of projects as a start:
If this works well, we can add more. The selection criteria should roughly be:
I think we could have a nested module for this, like
testdata/libraries/go.mod. Alibs.gofile would import at least one package from each module, to keepgo mod tidyhappy. We could then useGOPRIVATE='*' garble test allor somesuch, to run obfuscated versions of all tests in direct and indirect dependencies.Some notes:
-shortflag to save time, at least as a start.go test allsucceeds first, to reduce confusion if one of the tests is just broken.-short, for PRs.The text was updated successfully, but these errors were encountered: