Repository: metallb/metallb
Pull request #2224 (2024-01-02 16:54:38)

Bump tj-actions/changed-files from 40 to 41 in /.github/workflows
Bumps [tj-actions/changed-files](https://github.com/tj-actions/changed-files) from 40 to 41.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a href="https://github.com/tj-actions/changed-files/releases">tj-actions/changed-files's releases</a>.</em></p>
<blockquote>
<h2>v41</h2>
<h1>Changes in v41.0.1</h1>
<h2>What's Changed</h2>
<ul>
<li>Upgraded to v41 by <a href="https://github.com/tj-actions-bot"><code>@​tj-actions-bot</code></a> in <a href="https://redirect.github.com/tj-actions/changed-files/pull/1811">tj-actions/changed-files#1811</a></li>
<li>chore(deps): update dependency eslint-plugin-prettier to v5.1.2 by <a href="https://github.com/renovate"><code>@​renovate</code></a> in <a href="https://redirect.github.com/tj-actions/changed-files/pull/1813">tj-actions/changed-files#1813</a></li>
<li>fix: update characters escaped by safe output by <a href="https://github.com/jackton1"><code>@​jackton1</code></a> in <a href="https://redirect.github.com/tj-actions/changed-files/pull/1815">tj-actions/changed-files#1815</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a href="https://github.com/tj-actions/changed-files/compare/v41...v41.0.1">https://github.com/tj-actions/changed-files/compare/v41...v41.0.1</a></p>
<hr />
<h1>Changes in v41.0.0</h1>
<h2>🔥 🔥 BREAKING CHANGE 🔥 🔥</h2>
<p>A new <code>safe_output</code> input is now available to prevent outputting unsafe filename characters (Enabled by default). This would escape characters in the filename that could be used for command injection.</p>
<blockquote>
<p>[!NOTE]
This can be disabled by setting the <code>safe_output</code> to false this comes with a recommendation to store all outputs generated in an environment variable first before using them.</p>
</blockquote>
<h4>Example</h4>
<pre lang="yaml"><code>...
    - name: Get changed files
      id: changed-files
      uses: tj-actions/changed-files@v40
      with:
        safe_output: false # set to false because we are using an environment variable to store the output and avoid command injection.
<pre><code>- name: List all added files
  env:
    ADDED_FILES: ${{ steps.changed-files.outputs.added_files }}
  run: |
    for file in &amp;quot;$ADDED_FILES&amp;quot;; do
      echo &amp;quot;$file was added&amp;quot;
    done
</code></pre>
<p>...
</code></pre></p>
<h2>What's Changed</h2>
<ul>
<li>chore(deps): update typescript-eslint monorepo to v6.15.0 by <a href="https://github.com/renovate"><code>@​renovate</code></a> in <a href="https://redirect.github.com/tj-actions/changed-files/pull/1801">tj-actions/changed-files#1801</a></li>
<li>Upgraded to v40.2.3 by <a href="https://github.com/tj-actions-bot"><code>@​tj-actions-bot</code></a> in <a href="https://redirect.github.com/tj-actions/changed-files/pull/1800">tj-actions/changed-files#1800</a></li>
<li>chore(deps): update dependency eslint-plugin-prettier to v5.1.0 by <a href="https://github.com/renovate"><code>@​renovate</code></a> in <a href="https://redirect.github.com/tj-actions/changed-files/pull/1802">tj-actions/changed-files#1802</a></li>
<li>chore(deps): lock file maintenance by <a href="https://github.com/renovate"><code>@​renovate</code></a> in <a href="https://redirect.github.com/tj-actions/changed-files/pull/1803">tj-actions/changed-files#1803</a></li>
<li>chore(deps): update dependency eslint-plugin-prettier to v5.1.1 by <a href="https://github.com/renovate"><code>@​renovate</code></a> in <a href="https://redirect.github.com/tj-actions/changed-files/pull/1804">tj-actions/changed-files#1804</a></li>
<li>fix: update safe output regex and the docs by <a href="https://github.com/tj-actions-bot"><code>@​tj-actions-bot</code></a> in <a href="https://redirect.github.com/tj-actions/changed-files/pull/1805">tj-actions/changed-files#1805</a></li>
<li>Revert &quot;chore(deps): update actions/download-artifact action to v4&quot; by <a href="https://github.com/jackton1"><code>@​jackton1</code></a> in <a href="https://redirect.github.com/tj-actions/changed-files/pull/1806">tj-actions/changed-files#1806</a></li>
<li>Update README.md by <a href="https://github.com/jackton1"><code>@​jackton1</code></a> in <a href="https://redirect.github.com/tj-actions/changed-files/pull/1808">tj-actions/changed-files#1808</a></li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/tj-actions/changed-files/blob/main/HISTORY.md">tj-actions/changed-files's changelog</a>.</em></p>
<blockquote>
<h1>Changelog</h1>
<h1><a href="https://github.com/tj-actions/changed-files/compare/v41.0.0...v41.0.1">41.0.1</a> - (2023-12-24)</h1>
<h2><!-- raw HTML omitted -->🐛 Bug Fixes</h2>
<ul>
<li>Update characters escaped by safe output (<a href="https://redirect.github.com/tj-actions/changed-files/issues/1815">#1815</a>) (<a href="https://github.com/tj-actions/changed-files/commit/716b1e13042866565e00e85fd4ec490e186c4a2f">716b1e1</a>)  - (Tonye Jack)</li>
</ul>
<h2><!-- raw HTML omitted -->⚙️ Miscellaneous Tasks</h2>
<ul>
<li><strong>deps:</strong> Update dependency eslint-plugin-prettier to v5.1.2 (<a href="https://github.com/tj-actions/changed-files/commit/7aaf10d9eef19e8a2432a967b88124171152caaf">7aaf10d</a>)  - (renovate[bot])</li>
</ul>
<h2><!-- raw HTML omitted -->⬆️ Upgrades</h2>
<ul>
<li>Upgraded to v41 (<a href="https://redirect.github.com/tj-actions/changed-files/issues/1811">#1811</a>)</li>
</ul>
<p>Co-authored-by: jackton1 <a href="mailto:jackton1@users.noreply.github.com">jackton1@users.noreply.github.com</a> (<a href="https://github.com/tj-actions/changed-files/commit/cc08e170f4447237bcaf8acaacfa615b9cb86612">cc08e17</a>)  - (tj-actions[bot])</p>
<h1><a href="https://github.com/tj-actions/changed-files/compare/v40.2.3...v41.0.0">41.0.0</a> - (2023-12-23)</h1>
<h2><!-- raw HTML omitted -->🐛 Bug Fixes</h2>
<ul>
<li>Update safe output regex and the docs (<a href="https://redirect.github.com/tj-actions/changed-files/issues/1805">#1805</a>) (<a href="https://github.com/tj-actions/changed-files/commit/ff2f6e6b91913a7be42be1b5917330fe442f2ede">ff2f6e6</a>)  - (tj-actions[bot])</li>
</ul>
<h2><!-- raw HTML omitted -->⏪ Reverts</h2>
<ul>
<li>Revert &quot;chore(deps): update actions/download-artifact action to v4&quot; (<a href="https://redirect.github.com/tj-actions/changed-files/issues/1806">#1806</a>)</li>
</ul>
<p>(<a href="https://github.com/tj-actions/changed-files/commit/4f573fed06c9abb5da4c72f75c1c320718114ff7">4f573fe</a>)  - (Tonye Jack)</p>
<h2><!-- raw HTML omitted -->🔄 Update</h2>
<ul>
<li>Update README.md (<a href="https://github.com/tj-actions/changed-files/commit/6e79d6e3dbe48946636c2939c80ff5c84ff7f9fe">6e79d6e</a>)  - (Tonye Jack)</li>
<li>Update README.md (<a href="https://github.com/tj-actions/changed-files/commit/d13ac1942fb3c1d7d32017915bb082cebe8a272a">d13ac19</a>)  - (Tonye Jack)</li>
<li>Update README.md (<a href="https://github.com/tj-actions/changed-files/commit/bb89f97963be96b39e1a303e64d5b91a1af4c340">bb89f97</a>)  - (Tonye Jack)</li>
<li>Updated README.md (<a href="https://redirect.github.com/tj-actions/changed-files/issues/1810">#1810</a>)</li>
</ul>
<p>Co-authored-by: renovate[bot] <!-- raw HTML omitted --> (<a href="https://github.com/tj-actions/changed-files/commit/1864078d0afadf68ba489e671ecc09fefe8b70ab">1864078</a>)  - (tj-actions[bot])</p>
<ul>
<li>Update README.md (<a href="https://redirect.github.com/tj-actions/changed-files/issues/1808">#1808</a>)</li>
</ul>
<p>(<a href="https://github.com/tj-actions/changed-files/commit/47371c50e97c089212d9eb92ca26c8453224e78e">47371c5</a>)  - (Tonye Jack)</p>
<h2><!-- raw HTML omitted -->📝 Other</h2>
<ul>
<li>Merge pull request from GHSA-mcph-m25j-8j63</li>
</ul>
<ul>
<li>
<p>feat: add <code>safe_output</code> input enabled by default</p>
</li>
<li>
<p>fix: migrate README to safe uses of interpolation</p>
</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/tj-actions/changed-files/commit/716b1e13042866565e00e85fd4ec490e186c4a2f"><code>716b1e1</code></a> fix: update characters escaped by safe output (<a href="https://redirect.github.com/tj-actions/changed-files/issues/1815">#1815</a>)</li>
<li><a href="https://github.com/tj-actions/changed-files/commit/7aaf10d9eef19e8a2432a967b88124171152caaf"><code>7aaf10d</code></a> chore(deps): update dependency eslint-plugin-prettier to v5.1.2</li>
<li><a href="https://github.com/tj-actions/changed-files/commit/cc08e170f4447237bcaf8acaacfa615b9cb86612"><code>cc08e17</code></a> Upgraded to v41 (<a href="https://redirect.github.com/tj-actions/changed-files/issues/1811">#1811</a>)</li>
<li><a href="https://github.com/tj-actions/changed-files/commit/6e79d6e3dbe48946636c2939c80ff5c84ff7f9fe"><code>6e79d6e</code></a> Update README.md</li>
<li><a href="https://github.com/tj-actions/changed-files/commit/d13ac1942fb3c1d7d32017915bb082cebe8a272a"><code>d13ac19</code></a> Update README.md</li>
<li><a href="https://github.com/tj-actions/changed-files/commit/bb89f97963be96b39e1a303e64d5b91a1af4c340"><code>bb89f97</code></a> Update README.md</li>
<li><a href="https://github.com/tj-actions/changed-files/commit/1864078d0afadf68ba489e671ecc09fefe8b70ab"><code>1864078</code></a> Updated README.md (<a href="https://redirect.github.com/tj-actions/changed-files/issues/1810">#1810</a>)</li>
<li><a href="https://github.com/tj-actions/changed-files/commit/f495a0321d3fffa62da2573adf70b77d5eb2f57a"><code>f495a03</code></a> chore(deps): lock file maintenance</li>
<li><a href="https://github.com/tj-actions/changed-files/commit/47371c50e97c089212d9eb92ca26c8453224e78e"><code>47371c5</code></a> Update README.md (<a href="https://redirect.github.com/tj-actions/changed-files/issues/1808">#1808</a>)</li>
<li><a href="https://github.com/tj-actions/changed-files/commit/4f573fed06c9abb5da4c72f75c1c320718114ff7"><code>4f573fe</code></a> Revert &quot;chore(deps): update actions/download-artifact action to v4&quot; (<a href="https://redirect.github.com/tj-actions/changed-files/issues/1806">#1806</a>)</li>
<li>Additional commits viewable in <a href="https://github.com/tj-actions/changed-files/compare/v40...v41">compare view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=tj-actions/changed-files&package-manager=github_actions&previous-version=40&new-version=41)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/metallb/metallb/network/alerts).

</details>


Patch:
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index 18e60675..8fb48635 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -52,7 +52,7 @@ jobs:
 
       - name: Get changed files
         id: changed-files
-        uses: tj-actions/changed-files@v40
+        uses: tj-actions/changed-files@v41
 
       - name: Check if release notes where changed
         run: |
diff --git a/internal/config/validator.go b/internal/config/validator.go
index 2a215e67..a2895495 100644
--- a/internal/config/validator.go
+++ b/internal/config/validator.go
@@ -3,7 +3,7 @@
 package config
 
 import (
-	"errors"
+	"strings"
 
 	metallbv1beta1 "go.universe.tf/metallb/api/v1beta1"
 	metallbv1beta2 "go.universe.tf/metallb/api/v1beta2"
@@ -58,13 +58,43 @@ func (v *validator) Validate(resources ...client.ObjectList) error {
 			clusterResources.Nodes = append(clusterResources.Nodes, list.Items...)
 		}
 	}
+	clusterResources = resetTransientErrorsFields(clusterResources)
 	_, err := For(clusterResources, v.validate)
-	if errors.As(err, &TransientError{}) { // we do not want to make assumption on ordering in webhooks.
-		return nil
-	}
 	return err
 }
 
 func NewValidator(validate Validate) apivalidate.ClusterObjects {
 	return &validator{validate: validate}
 }
+
+// Returns the given ClusterResources without the fields that can cause a TransientError.
+// We use this so the webhooks do not make assumptions based on the ordering of objects.
+func resetTransientErrorsFields(clusterResources ClusterResources) ClusterResources {
+	for i := range clusterResources.Peers {
+		clusterResources.Peers[i].Spec.BFDProfile = ""
+		clusterResources.Peers[i].Spec.PasswordSecret = v1.SecretReference{}
+	}
+	for i, bgpAdv := range clusterResources.BGPAdvs {
+		var communities []string
+		for _, community := range bgpAdv.Spec.Communities {
+			// We want to pass only communities that are potentially explicit values.
+			if strings.Contains(community, ":") {
+				communities = append(communities, community)
+			}
+		}
+		clusterResources.BGPAdvs[i].Spec.Communities = communities
+	}
+	for i, legacyAddrPool := range clusterResources.LegacyAddressPools {
+		for j, bgpAdv := range legacyAddrPool.Spec.BGPAdvertisements {
+			var communities []string
+			for k := range bgpAdv.Communities {
+				// We want to pass only communities that are potentially explicit values.
+				if strings.Contains(bgpAdv.Communities[k], ":") {
+					communities = append(communities, bgpAdv.Communities[k])
+				}
+			}
+			clusterResources.LegacyAddressPools[i].Spec.BGPAdvertisements[j].Communities = communities
+		}
+	}
+	return clusterResources
+}
diff --git a/website/content/concepts/layer2.md b/website/content/concepts/layer2.md
index 7dedab2b..9d999cc8 100644
--- a/website/content/concepts/layer2.md
+++ b/website/content/concepts/layer2.md
@@ -82,7 +82,7 @@ announcing a given IP.
 
 ### Adding or removing nodes
 
-Given the leader election algoritm described above, removing a node does not change the
+Given the leader election algorithm described above, removing a node does not change the
 speaker announcing the VIP, while adding a node will change it only if it becomes the new
 first element of the list.
 
diff --git a/website/content/release-notes/_index.md b/website/content/release-notes/_index.md
index ead88371..cba55bbd 100644
--- a/website/content/release-notes/_index.md
+++ b/website/content/release-notes/_index.md
@@ -17,6 +17,7 @@ Chores:
 - Images updated to Go 1.20.12 ([PR 2213](https://github.com/metallb/metallb/pull/2213))
 - CI/E2E: Relabel the frr metrics from frr-k8s to show as MetalLB's ([PR 2210](https://github.com/metallb/metallb/pull/2210))
 - Dev-env: change the default BGP mode to FRR ([PR 2196](https://github.com/metallb/metallb/pull/2196))
+- Webhooks: avoid transient errors ([PR 2202](https://github.com/metallb/metallb/pull/2202)), [ISSUE 2173](https://github.com/metallb/metallb/issues/2173))
 
 ## Version 0.13.12
 


Test patch:
diff --git a/internal/config/validator_test.go b/internal/config/validator_test.go
index 4b618714..50e142c0 100644
--- a/internal/config/validator_test.go
+++ b/internal/config/validator_test.go
@@ -5,16 +5,19 @@ package config
 import (
 	"testing"
 
-	"go.universe.tf/metallb/api/v1beta2"
+	"github.com/google/go-cmp/cmp"
+	metallbv1beta1 "go.universe.tf/metallb/api/v1beta1"
+	metallbv1beta2 "go.universe.tf/metallb/api/v1beta2"
+	v1 "k8s.io/api/core/v1"
 )
 
 func TestValidator(t *testing.T) {
 	v := validator{DontValidate}
 
-	bgpPeerList := v1beta2.BGPPeerList{
-		Items: []v1beta2.BGPPeer{
+	bgpPeerList := metallbv1beta2.BGPPeerList{
+		Items: []metallbv1beta2.BGPPeer{
 			{
-				Spec: v1beta2.BGPPeerSpec{
+				Spec: metallbv1beta2.BGPPeerSpec{
 					MyASN:      42,
 					ASN:        42,
 					Address:    "1.2.3.4",
@@ -28,3 +31,131 @@ func TestValidator(t *testing.T) {
 		t.Error("The validator should not fail for non existing bfd profile")
 	}
 }
+
+func TestResetTransientErrorsFields(t *testing.T) {
+	tests := []struct {
+		desc             string
+		clusterResources ClusterResources
+		expected         ClusterResources
+	}{
+		{
+			desc:             "empty resource",
+			clusterResources: ClusterResources{},
+			expected:         ClusterResources{},
+		},
+		{
+			desc: "BFDProfiles, PasswordSecrets and Community references are reset",
+			clusterResources: ClusterResources{
+				Peers: []metallbv1beta2.BGPPeer{
+					{
+						Spec: metallbv1beta2.BGPPeerSpec{
+							BFDProfile: "myBFDProfile",
+							MyASN:      64512,
+							ASN:        64512,
+							Address:    "172.30.0.3",
+							PasswordSecret: v1.SecretReference{
+								Name:      "myName",
+								Namespace: "myNamespace",
+							},
+						},
+					},
+				},
+				Communities: []metallbv1beta1.Community{
+					{
+						Spec: metallbv1beta1.CommunitySpec{
+							Communities: []metallbv1beta1.CommunityAlias{
+								{
+									Name:  "myCommunityAlias",
+									Value: "65000:100",
+								},
+							},
+						},
+					},
+				},
+				BGPAdvs: []metallbv1beta1.BGPAdvertisement{
+					{
+						Spec: metallbv1beta1.BGPAdvertisementSpec{
+							Communities: []string{
+								"11111:11aaaa",
+								"larg:12345:12345:12345",
+								"alias",
+								"myCommunityAlias",
+							},
+						},
+					},
+				},
+				LegacyAddressPools: []metallbv1beta1.AddressPool{
+					{
+						Spec: metallbv1beta1.AddressPoolSpec{
+							BGPAdvertisements: []metallbv1beta1.LegacyBgpAdvertisement{
+								{
+									Communities: []string{
+										"11111:11aaaa",
+										"larg:12345:12345:12345",
+										"alias",
+										"myCommunityAlias",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: ClusterResources{
+				Peers: []metallbv1beta2.BGPPeer{
+					{
+						Spec: metallbv1beta2.BGPPeerSpec{
+							BFDProfile:     "",
+							MyASN:          64512,
+							ASN:            64512,
+							Address:        "172.30.0.3",
+							PasswordSecret: v1.SecretReference{},
+						},
+					},
+				},
+				Communities: []metallbv1beta1.Community{
+					{
+						Spec: metallbv1beta1.CommunitySpec{
+							Communities: []metallbv1beta1.CommunityAlias{
+								{
+									Name:  "myCommunityAlias",
+									Value: "65000:100",
+								},
+							},
+						},
+					},
+				},
+				BGPAdvs: []metallbv1beta1.BGPAdvertisement{
+					{
+						Spec: metallbv1beta1.BGPAdvertisementSpec{
+							Communities: []string{
+								"11111:11aaaa",
+								"larg:12345:12345:12345",
+							},
+						},
+					},
+				},
+				LegacyAddressPools: []metallbv1beta1.AddressPool{
+					{
+						Spec: metallbv1beta1.AddressPoolSpec{
+							BGPAdvertisements: []metallbv1beta1.LegacyBgpAdvertisement{
+								{
+									Communities: []string{
+										"11111:11aaaa",
+										"larg:12345:12345:12345",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		result := resetTransientErrorsFields(test.clusterResources)
+		if !cmp.Equal(result, test.expected) {
+			t.Fatalf("test %s failed, unexpected clusterResources (-want +got)\n%s", test.desc, cmp.Diff(result, test.expected))
+		}
+	}
+}