From 1fb63eb5f0314c5aad975473dc12a4e16138c66b Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Mon, 28 Aug 2023 12:09:06 +0200
Subject: [PATCH] opts: ParseRestartPolicy: improve validation of max
 restart-counts

Use the new container.ValidateRestartPolicy utility to verify if a max-restart-count
is allowed for the given restart-policy.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 cli/command/container/opts_test.go  | 15 ++++++++++-----
 cli/compose/convert/service.go      |  5 ++---
 cli/compose/convert/service_test.go |  2 +-
 opts/parse.go                       |  3 +++
 4 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/cli/command/container/opts_test.go b/cli/command/container/opts_test.go
index 51a0e72b4943..6acf8f51aef7 100644
--- a/cli/command/container/opts_test.go
+++ b/cli/command/container/opts_test.go
@@ -819,6 +819,10 @@ func TestParseRestartPolicy(t *testing.T) {
 				Name: container.RestartPolicyDisabled,
 			},
 		},
+		{
+			input:       "no:1",
+			expectedErr: "invalid restart policy: maximum retry count can only be used with 'on-failure'",
+		},
 		{
 			input:       ":1",
 			expectedErr: "invalid restart policy format: no policy provided before colon",
@@ -830,11 +834,8 @@ func TestParseRestartPolicy(t *testing.T) {
 			},
 		},
 		{
-			input: "always:1",
-			expected: container.RestartPolicy{
-				Name:              container.RestartPolicyAlways,
-				MaximumRetryCount: 1,
-			},
+			input:       "always:1",
+			expectedErr: "invalid restart policy: maximum retry count can only be used with 'on-failure'",
 		},
 		{
 			input:       "always:2:3",
@@ -857,6 +858,10 @@ func TestParseRestartPolicy(t *testing.T) {
 				Name: container.RestartPolicyUnlessStopped,
 			},
 		},
+		{
+			input:       "unless-stopped:1",
+			expectedErr: "invalid restart policy: maximum retry count can only be used with 'on-failure'",
+		},
 		{
 			input:       "unless-stopped:invalid",
 			expectedErr: "invalid restart policy format: maximum retry count must be an integer",
diff --git a/cli/compose/convert/service.go b/cli/compose/convert/service.go
index 5fbc289d830e..8a7885765731 100644
--- a/cli/compose/convert/service.go
+++ b/cli/compose/convert/service.go
@@ -80,8 +80,7 @@ func Service(
 		return swarm.ServiceSpec{}, err
 	}
 
-	restartPolicy, err := convertRestartPolicy(
-		service.Restart, service.Deploy.RestartPolicy)
+	restartPolicy, err := convertRestartPolicy(service.Restart, service.Deploy.RestartPolicy)
 	if err != nil {
 		return swarm.ServiceSpec{}, err
 	}
@@ -484,7 +483,7 @@ func convertRestartPolicy(restart string, source *composetypes.RestartPolicy) (*
 				MaxAttempts: &attempts,
 			}, nil
 		default:
-			return nil, fmt.Errorf("unknown restart policy: %s", restart)
+			return nil, fmt.Errorf("invalid restart policy: unknown policy '%s'", restart)
 		}
 	}
 
diff --git a/cli/compose/convert/service_test.go b/cli/compose/convert/service_test.go
index b5999fe6ea8e..7f990f304341 100644
--- a/cli/compose/convert/service_test.go
+++ b/cli/compose/convert/service_test.go
@@ -26,7 +26,7 @@ func TestConvertRestartPolicyFromNone(t *testing.T) {
 
 func TestConvertRestartPolicyFromUnknown(t *testing.T) {
 	_, err := convertRestartPolicy("unknown", nil)
-	assert.Error(t, err, "unknown restart policy: unknown")
+	assert.Error(t, err, "invalid restart policy: unknown policy 'unknown'; use one of 'no', 'always', 'on-failure', or 'unless-stopped'")
 }
 
 func TestConvertRestartPolicyFromAlways(t *testing.T) {
diff --git a/opts/parse.go b/opts/parse.go
index c04fc7d4b8bb..57ffa6d8b9e1 100644
--- a/opts/parse.go
+++ b/opts/parse.go
@@ -93,5 +93,8 @@ func ParseRestartPolicy(policy string) (container.RestartPolicy, error) {
 	}
 
 	p.Name = container.RestartPolicyMode(k)
+	if err := container.ValidateRestartPolicy(p); err != nil {
+		return container.RestartPolicy{}, err
+	}
 	return p, nil
 }