From 53fe469b2c8690f5ca57aafa954c7075b959eaf9 Mon Sep 17 00:00:00 2001 From: mwindower Date: Tue, 8 Dec 2020 10:12:08 +0100 Subject: [PATCH] Prepare for firewall-controller self-update release (#69) --- api/v1/firewall_types.go | 4 +- api/v1/firewall_types_test.go | 135 ++++++++++++++++++++++++++++++++++ pkg/updater/updater.go | 12 ++- 3 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 api/v1/firewall_types_test.go diff --git a/api/v1/firewall_types.go b/api/v1/firewall_types.go index 308542b7..47e8dc50 100644 --- a/api/v1/firewall_types.go +++ b/api/v1/firewall_types.go @@ -56,6 +56,8 @@ type FirewallSpec struct { // Signature of firewall attributes generated by GEPM. Signature string `json:"signature"` + // ControllerVersion holds the firewall-controller version to reconcile. + ControllerVersion string `json:"controllerVersion,omitempty"` } // Data contains the fields over which the signature is calculated. @@ -76,8 +78,6 @@ type Data struct { EgressRules []EgressRuleSNAT `json:"egressRules,omitempty"` // FirewallNetworks holds the networks known at the metal-api for this firewall machine FirewallNetworks []FirewallNetwork `json:"firewallNetworks,omitempty"` - // ControllerVersion holds the firewall-controller version to reconcile. - ControllerVersion string `json:"controllerVersion,omitempty"` } // FirewallStatus defines the observed state of Firewall diff --git a/api/v1/firewall_types_test.go b/api/v1/firewall_types_test.go new file mode 100644 index 00000000..da4e59fa --- /dev/null +++ b/api/v1/firewall_types_test.go @@ -0,0 +1,135 @@ +/* + + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1 + +import ( + "encoding/json" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestSerialization(t *testing.T) { + asn := int64(123456) + internet := "internet" + external := "external" + trueFlag := true + vrf := int64(123456) + + tests := []struct { + name string + data *Data + want string + wantErr bool + }{ + { + name: "test for api breaking changes in firewall spec", + data: &Data{ + Interval: "10s", + DryRun: false, + Ipv4RuleFile: "/etc/nftables/firewall-controller.ipv4", + RateLimits: []RateLimit{ + { + NetworkID: "internet", + Rate: 10, + }, + }, + InternalPrefixes: []string{"10.0.0.0/8"}, + EgressRules: []EgressRuleSNAT{ + { + NetworkID: "internet", + IPs: []string{"1.2.3.4"}, + }, + }, + FirewallNetworks: []FirewallNetwork{ + { + Asn: &asn, + Networkid: &internet, + Destinationprefixes: []string{"0.0.0.0/0"}, + Networktype: &external, + Nat: &trueFlag, + Vrf: &vrf, + Ips: []string{"1.2.3.4"}, + Prefixes: []string{"1.2.3.0/24"}, + }, + }, + }, + want: `{ + "interval": "10s", + "ipv4rulefile": "/etc/nftables/firewall-controller.ipv4", + "rateLimits": [ + { + "networkid": "internet", + "rate": 10 + } + ], + "internalprefixes": [ + "10.0.0.0/8" + ], + "egressRules": [ + { + "networkid": "internet", + "ips": [ + "1.2.3.4" + ] + } + ], + "firewallNetworks": [ + { + "asn": 123456, + "destinationprefixes": [ + "0.0.0.0/0" + ], + "ips": [ + "1.2.3.4" + ], + "nat": true, + "networkid": "internet", + "networktype": "external", + "prefixes": [ + "1.2.3.0/24" + ], + "vrf": 123456 + } + ] +}`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b, err := json.MarshalIndent(tt.data, "", " ") + if err != nil { + t.Error(err) + t.Fail() + } + + if !cmp.Equal(string(b), tt.want) { + // CAUTION! This is really bad! + // + // This means that you introduced incompatible changes to the FirewallData struct (= change in the api) + // This breaks the contract btw. gardener-extension-provider-metal and firewall-controller: + // - a firewall-controller version with such changes will not be able to verify signatures generated by an older gepm version + // - or other way round: newer gepm versions will generate a signature with additional fields that are not known to older firewall-controller versions potentially out there (in older metal-images) + // + // You may consider + // - if you added an additional field to FirewallData: annotate it with omitempty + // - publish a new major release of firewall-controller and rolling all cluster firewalls + t.Errorf("json marshalling yields diff - this breaks the api btw. : %s", cmp.Diff(string(b), tt.want)) + } + }) + } +} diff --git a/pkg/updater/updater.go b/pkg/updater/updater.go index 49981567..77402fa7 100644 --- a/pkg/updater/updater.go +++ b/pkg/updater/updater.go @@ -37,12 +37,12 @@ func UpdateToSpecVersion(f firewallv1.Firewall, log logr.Logger, recorder record } recorder.Eventf(&f, "Normal", "Self-Reconcilation", "replacing firewall-controller version %s with version %s", v.Version, f.Spec.ControllerVersion) - asset, err := determineGithubAsset(f.Spec.ControllerVersion) + asset, err := DetermineGithubAsset(f.Spec.ControllerVersion) if err != nil { return err } - binaryReader, checksum, err := fetchGithubAssetAndChecksum(asset) + binaryReader, checksum, err := FetchGithubAssetAndChecksum(asset) if err != nil { return fmt.Errorf("could not fetch github asset and checksum for firewall-controller version %s, err: %w", f.Spec.ControllerVersion, err) } @@ -59,7 +59,7 @@ func UpdateToSpecVersion(f firewallv1.Firewall, log logr.Logger, recorder record return nil } -func determineGithubAsset(githubTag string) (*github.ReleaseAsset, error) { +func DetermineGithubAsset(githubTag string) (*github.ReleaseAsset, error) { client := github.NewClient(nil) releases, _, err := client.Repositories.ListReleases(context.Background(), gitHubOwner, gitHubRepo, &github.ListOptions{}) if err != nil { @@ -74,6 +74,10 @@ func determineGithubAsset(githubTag string) (*github.ReleaseAsset, error) { } } + if rel == nil { + return nil, fmt.Errorf("could not find release with tag %s", githubTag) + } + var asset *github.ReleaseAsset for _, ra := range rel.Assets { if ra.GetName() == gitHubArtifact { @@ -88,7 +92,7 @@ func determineGithubAsset(githubTag string) (*github.ReleaseAsset, error) { return asset, nil } -func fetchGithubAssetAndChecksum(ra *github.ReleaseAsset) (io.ReadCloser, string, error) { +func FetchGithubAssetAndChecksum(ra *github.ReleaseAsset) (io.ReadCloser, string, error) { checksum, err := slurpFile(ra.GetBrowserDownloadURL() + ".sha256") if err != nil { return nil, "", fmt.Errorf("could not slurp checksum file for asset %s, err: %w", ra.GetBrowserDownloadURL(), err)