snap, snap/pack: add pack validation for default-configure hook

[ernestl]

Additional to PR-12701, to address this comment.

As a temporary measure checkConfigureHooks was added which extends checkSnap that is called from doMountSnap

The requirement is to have a similar check in place in:

• snap.Validate (with this in place, arguably the temporary measure can fall away)
• snap pack loadAndValidate

This PR proposes to achieve this by:

1. Export addImplicitHooksFromContainer and use in loadAndValidate to populate missing implicit hooks to enable validation.
2. Extend snap.Validate hook validation to check for the invalid combination of default-configure without configure.

Further consideration:

• Arguably we can remove the temporary measure, but the doMountSnap handler checkSnap uses snap.ReadInfoFromMountPoint which unlike its counterpart snap.ReadInfoFromSnapFile does not have a snap.Validation step included. This means without the temporary measure in place, we would loose the check at that stage.
• It seems as if direct snap pack testing is limited to a fairly simple case:
  Propose to extend test to include repack of some of the most complicated snaps we have, including essential snaps.

[codecov-commenter]

Codecov Report

Merging #13097 (f4f85e9) into master (d8942e6) will increase coverage by 0.00%.
Report is 2 commits behind head on master.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more.

@@           Coverage Diff           @@
##           master   #13097   +/-   ##
=======================================
  Coverage   78.80%   78.80%           
=======================================
  Files        1011     1011           
  Lines      125673   125691   +18     
=======================================
+ Hits        99032    99056   +24     
+ Misses      20433    20429    -4     
+ Partials     6208     6206    -2     

Flag	Coverage Δ
unittests	78.80% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
snap/pack/pack.go	80.15% <100.00%> (+8.97%)	⬆️
snap/validate.go	95.30% <100.00%> (+0.06%)	⬆️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[Meulengracht]

Thanks, I think this looks super nice :)

[pedronis]

@ernestl do you need to merge master to fix golangci-lint issues?

[pedronis]

direction looks reasonable, some wonderings

[ernestl]

@ernestl do you need to merge master to fix golangci-lint issues?

Yes this was required, I realise it was not ideal to rebase, my apologies.

[ernestl]

I attempted approach to create /internal helpers and share code with ReadInfoFromSnapFile, but concluded it would not look/work nice. As far as I can see not possible without exporting new function, so exporting AddImplicitHooksFromContainer seems like an ok trade-off.

[pedronis]

thanks

[Meulengracht]

Thank you