-
Notifications
You must be signed in to change notification settings - Fork 264
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
Add firewall extension decompiler, make firewall modifications work for msi changes #431
Conversation
Is it possible to unit test this ? I added some changes to enable nesting firewall exceptions under Looks like the built in testing framework only allows testing a single extension at a time. Would this have to be covered by an end to end test? |
Unit tests can only validate that the correct data gets into the MSI. Integration (end-to-end) tests are required to execution of the CustomAction. There is such a test for some user scenarios. |
Or just add support for more than one extension. I'm sure this isn't the only place it could be used. |
The firewall extension is referenced as a project in the I don't know if that makes sense, or would be ok. |
It wouldn't be the only one; NetFx depends on both Bal and Util. |
Ah, the production extension itself has the dependencies. |
It does use PackageReference, not ProjectReference, as I recall. |
9fb8816
to
3565918
Compare
Where would be the best place to add integration tests for the firewall extension? |
src\test\msi\WixToolsetTest.MsiE2E. The Util.wixext tests live there so Firewall.wixext tests should be neighbors. |
I will review this, add integration tests and rebase with the develop branch after Windows XP support has been removed from the firewall extension. Might be able to split this into multiple PRs so they're easier to review as well |
Because you're adding columns to the custom table, this will need a bit of triage discussion: I think we need to change the |
No problem. |
3565918
to
9d89238
Compare
Added more firewall attributes. inetfwrule1 and inetfwrule2 interface is all covered, but needs more E2E tests |
39820ef
to
c8641c6
Compare
Would you be able to approve the build workflow on this? Specifically, I'm not sure how many network interfaces there will be on the build image. |
c8641c6
to
90a28a1
Compare
Woops! Let's try that again. |
3f12d6d
to
51808f8
Compare
Added more compiler/decompiler tests. I think I'm pretty much done code wise. What's the next step to help get this merged ? |
At the moment, it's #451. I don't expect an immediate resolution, so if it's otherwise ready, you should look at building the v4 packages and checking them in. |
51808f8
to
85cc45c
Compare
Is this what you mean? |
Looks right (probably don't need the .wixpdb). If we end up supporting v4 and v5 together, we could go back to building them individually, but it's likely we'll have Data or Extensibility changes that will prevent it. |
85cc45c
to
685bb52
Compare
685bb52
to
7475716
Compare
rebase + added E2E tests covering msi properties and OnChange flag. I think there is enough test coverage in there now. |
Ready for review? Any other PRs need updating? |
Yes, ready for review. I've closed off some redundant PRs and rebased #451, which might still be useful |
Cool, thanks. I will gird my loins and jump in, probably next weekend. |
@chrisbednarski, can you apply this patch? I don't have permission to push to your branch. Checks out locally but I want the PR CI build, which I can't trigger for some reason. I'm hoping another commit will work. |
7475716
to
23686f5
Compare
applied the patch |
23686f5
to
ecb5618
Compare
and rebased with develop branch |
Very cool! Thanks for contributing (and working through everything). |
If you get very bored, the schema doc at https://github.com/wixtoolset/web/blob/master/src/xsd4/firewall.xsd could use doc on the new features. |
Add firewall extension decompiler.
MSI database schema changes:
Direction
nullableCompiler changes:
the firewall element can be nested under ServiceInstall or util:ServiceConfig
Added
OnUpdate
property to the Wix firewall language which affect what happens during MSI modifications and repairs:EnableOnly
- enable existing firewall rules, do not change any other properties. This is similar to how wix V4 firewall CA worksDoNothing
- do not update any properties of existing firewall rulesdefault: re-assign all properties on existing firewall rules
Added columns for all firewall properties supported by Windows Vista/7/8+
INetFwRule Windows Vista / Server 2008
INetFwRule2 Windows 7 / Server 2008 R2
INetFwRule3 Windows 8/ Server 2012
The related COM interfaces are only queried when at least one of the properties of the firewall interface has been set by the wix compiler.
IE. If a firewall rule is created and none of the
INetFwRule2
properties are set, the firewall rule can be deployed on Vista/7/8+ . Same withINetFwRule3
properties. If none are used, the firewall rule can be deployed on Vista/7/8+When a
INetFwRule2
property is added to the firewall rule, it will no longer work on Vista, only on 7/8+This is handled by the
feaAddINetFwRule2
andfeaAddINetFwRule3
flags in theAttributes
column.