mask helm conflict errors (#1016)

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
Co-authored-by: Per Goncalves da Silva <pegoncal@redhat.com>

cmd/manager/main.go

@@ -41,6 +41,7 @@ import (
 	helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
 
 	ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
+	"github.com/operator-framework/operator-controller/internal/action"
 	"github.com/operator-framework/operator-controller/internal/catalogmetadata/cache"
 	catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client"
 	"github.com/operator-framework/operator-controller/internal/controllers"
@@ -184,9 +185,10 @@ func main() {
 		os.Exit(1)
 	}
 
-	acg, err := helmclient.NewActionClientGetter(cfgGetter,
+	acg, err := action.NewWrappedActionClientGetter(cfgGetter,
 		helmclient.WithFailureRollbacks(false),
 	)
+
 	if err != nil {
 		setupLog.Error(err, "unable to create helm client")
 		os.Exit(1)

internal/action/error/errors.go

@@ -0,0 +1,73 @@
+package error
+
+import (
+	"fmt"
+	"regexp"
+)
+
+var (
+	installConflictErrorPattern = regexp.MustCompile(`Unable to continue with install: (\w+) "(.*)" in namespace "(.*)" exists and cannot be imported into the current release.*`)
+)
+
+type Olmv1Err struct {
+	originalErr error
+	message     string
+}
+
+func (o Olmv1Err) Error() string {
+	return o.message
+}
+
+func (o Olmv1Err) Cause() error {
+	return o.originalErr
+}
+
+func newOlmv1Err(originalErr error, message string) error {
+	return &Olmv1Err{
+		originalErr: originalErr,
+		message:     message,
+	}
+}
+
+func AsOlmErr(originalErr error) error {
+	if originalErr == nil {
+		return nil
+	}
+
+	for _, exec := range rules {
+		if err := exec(originalErr); err != nil {
+			return err
+		}
+	}
+
+	// let's mark any unmatched errors as unknown
+	return defaultErrTranslator(originalErr)
+}
+
+// rule is a function that translates an error into a more specific error
+// typically to hide internal implementation details
+// in: helm error
+// out: nil -> no translation | !nil -> translated error
+type rule func(originalErr error) error
+
+// rules is a list of rules for error translation
+var rules = []rule{
+	helmInstallConflictErr,
+}
+
+// installConflictErrorTranslator
+func helmInstallConflictErr(originalErr error) error {
+	matches := installConflictErrorPattern.FindStringSubmatch(originalErr.Error())
+	if len(matches) != 4 {
+		// there was no match
+		return nil
+	}
+	kind := matches[1]
+	name := matches[2]
+	namespace := matches[3]
+	return newOlmv1Err(originalErr, fmt.Sprintf("%s '%s' already exists in namespace '%s' and cannot be managed by operator-controller", kind, name, namespace))
+}
+
+func defaultErrTranslator(originalErr error) error {
+	return originalErr
+}

internal/action/error/errors_test.go

@@ -0,0 +1,44 @@
+package error
+
+import (
+	"errors"
+	"testing"
+)
+
+func TestAsOlmErr(t *testing.T) {
+	tests := []struct {
+		name     string
+		err      error
+		expected error
+	}{
+		{
+			name:     "Install conflict error (match)",
+			err:      errors.New("Unable to continue with install: Deployment \"my-deploy\" in namespace \"my-namespace\" exists and cannot be imported into the current release"),
+			expected: errors.New("Deployment 'my-deploy' already exists in namespace 'my-namespace' and cannot be managed by operator-controller"),
+		},
+		{
+			name:     "Install conflict error (no match)",
+			err:      errors.New("Unable to continue with install: because of something"),
+			expected: errors.New("Unable to continue with install: because of something"),
+		},
+		{
+			name:     "Unknown error",
+			err:      errors.New("some unknown error"),
+			expected: errors.New("some unknown error"),
+		},
+		{
+			name:     "Nil error",
+			err:      nil,
+			expected: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := AsOlmErr(tt.err)
+			if result != nil && result.Error() != tt.expected.Error() {
+				t.Errorf("Expected: %v, got: %v", tt.expected, result)
+			}
+		})
+	}
+}

internal/action/helm.go

@@ -0,0 +1,80 @@
+package action
+
+import (
+	"context"
+
+	"helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/release"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	actionclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
+
+	olmv1error "github.com/operator-framework/operator-controller/internal/action/error"
+)
+
+type ActionClientGetter struct {
+	actionclient.ActionClientGetter
+}
+
+func (a ActionClientGetter) ActionClientFor(ctx context.Context, obj client.Object) (actionclient.ActionInterface, error) {
+	ac, err := a.ActionClientGetter.ActionClientFor(ctx, obj)
+	if err != nil {
+		return nil, err
+	}
+	return &ActionClient{
+		ActionInterface:             ac,
+		actionClientErrorTranslator: olmv1error.AsOlmErr,
+	}, nil
+}
+
+func NewWrappedActionClientGetter(acg actionclient.ActionConfigGetter, opts ...actionclient.ActionClientGetterOption) (actionclient.ActionClientGetter, error) {
+	ag, err := actionclient.NewActionClientGetter(acg, opts...)
+	if err != nil {
+		return nil, err
+	}
+	return &ActionClientGetter{
+		ActionClientGetter: ag,
+	}, nil
+}
+
+type ActionClientErrorTranslator func(err error) error
+
+type ActionClient struct {
+	actionclient.ActionInterface
+	actionClientErrorTranslator ActionClientErrorTranslator
+}
+
+func NewWrappedActionClient(ca actionclient.ActionInterface, errTranslator ActionClientErrorTranslator) actionclient.ActionInterface {
+	return &ActionClient{
+		ActionInterface:             ca,
+		actionClientErrorTranslator: errTranslator,
+	}
+}
+
+func (a ActionClient) Install(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...actionclient.InstallOption) (*release.Release, error) {
+	rel, err := a.ActionInterface.Install(name, namespace, chrt, vals, opts...)
+	err = a.actionClientErrorTranslator(err)
+	return rel, err
+}
+
+func (a ActionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...actionclient.UpgradeOption) (*release.Release, error) {
+	rel, err := a.ActionInterface.Upgrade(name, namespace, chrt, vals, opts...)
+	err = a.actionClientErrorTranslator(err)
+	return rel, err
+}
+
+func (a ActionClient) Uninstall(name string, opts ...actionclient.UninstallOption) (*release.UninstallReleaseResponse, error) {
+	resp, err := a.ActionInterface.Uninstall(name, opts...)
+	err = a.actionClientErrorTranslator(err)
+	return resp, err
+}
+
+func (a ActionClient) Get(name string, opts ...actionclient.GetOption) (*release.Release, error) {
+	resp, err := a.ActionInterface.Get(name, opts...)
+	err = a.actionClientErrorTranslator(err)
+	return resp, err
+}
+
+func (a ActionClient) Reconcile(rel *release.Release) error {
+	return a.actionClientErrorTranslator(a.ActionInterface.Reconcile(rel))
+}

internal/action/helm_test.go

@@ -0,0 +1,141 @@
+package action
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/release"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	actionclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
+)
+
+var _ actionclient.ActionInterface = &mockActionClient{}
+
+type mockActionClient struct {
+	mock.Mock
+}
+
+func (m *mockActionClient) Get(name string, opts ...actionclient.GetOption) (*release.Release, error) {
+	args := m.Called(name, opts)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(*release.Release), args.Error(1)
+}
+
+func (m *mockActionClient) Install(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...actionclient.InstallOption) (*release.Release, error) {
+	args := m.Called(name, namespace, chrt, vals, opts)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(*release.Release), args.Error(1)
+}
+
+func (m *mockActionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...actionclient.UpgradeOption) (*release.Release, error) {
+	args := m.Called(name, namespace, chrt, vals, opts)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(*release.Release), args.Error(1)
+}
+
+func (m *mockActionClient) Uninstall(name string, opts ...actionclient.UninstallOption) (*release.UninstallReleaseResponse, error) {
+	args := m.Called(name, opts)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(*release.UninstallReleaseResponse), args.Error(1)
+}
+
+func (m *mockActionClient) Reconcile(rel *release.Release) error {
+	args := m.Called(rel)
+	return args.Error(0)
+}
+
+var _ actionclient.ActionClientGetter = &mockActionClientGetter{}
+
+type mockActionClientGetter struct {
+	mock.Mock
+}
+
+func (m *mockActionClientGetter) ActionClientFor(ctx context.Context, obj client.Object) (actionclient.ActionInterface, error) {
+	args := m.Called(ctx, obj)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(actionclient.ActionInterface), args.Error(1)
+}
+
+func TestActionClientErrorTranslation(t *testing.T) {
+	originalError := fmt.Errorf("some error")
+	expectedErr := fmt.Errorf("something other error")
+	errTranslator := func(originalErr error) error {
+		return expectedErr
+	}
+
+	ac := new(mockActionClient)
+	ac.On("Get", mock.Anything, mock.Anything).Return(nil, originalError)
+	ac.On("Install", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, originalError)
+	ac.On("Uninstall", mock.Anything, mock.Anything).Return(nil, originalError)
+	ac.On("Upgrade", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, originalError)
+	ac.On("Reconcile", mock.Anything, mock.Anything).Return(originalError)
+
+	wrappedAc := NewWrappedActionClient(ac, errTranslator)
+
+	// Get
+	_, err := wrappedAc.Get("something")
+	assert.Equal(t, expectedErr, err, "expected Get() to return translated error")
+
+	// Install
+	_, err = wrappedAc.Install("something", "somethingelse", nil, nil)
+	assert.Equal(t, expectedErr, err, "expected Install() to return translated error")
+
+	// Uninstall
+	_, err = wrappedAc.Uninstall("something")
+	assert.Equal(t, expectedErr, err, "expected Uninstall() to return translated error")
+
+	// Upgrade
+	_, err = wrappedAc.Upgrade("something", "somethingelse", nil, nil)
+	assert.Equal(t, expectedErr, err, "expected Upgrade() to return translated error")
+
+	// Reconcile
+	err = wrappedAc.Reconcile(nil)
+	assert.Equal(t, expectedErr, err, "expected Reconcile() to return translated error")
+}
+
+func TestActionClientFor(t *testing.T) {
+	// Create a mock for the ActionClientGetter
+	mockActionClientGetter := new(mockActionClientGetter)
+	mockActionInterface := new(mockActionClient)
+	testError := errors.New("test error")
+
+	// Set up expectations for the mock
+	mockActionClientGetter.On("ActionClientFor", mock.Anything, mock.Anything).Return(mockActionInterface, nil).Once()
+	mockActionClientGetter.On("ActionClientFor", mock.Anything, mock.Anything).Return(nil, testError).Once()
+
+	// Create an instance of ActionClientGetter with the mock
+	acg := ActionClientGetter{
+		ActionClientGetter: mockActionClientGetter,
+	}
+
+	// Define a test context and object
+	ctx := context.Background()
+	var obj client.Object // Replace with an actual client.Object implementation
+
+	// Test the successful case
+	actionClient, err := acg.ActionClientFor(ctx, obj)
+	assert.NoError(t, err)
+	assert.NotNil(t, actionClient)
+	assert.IsType(t, &ActionClient{}, actionClient)
+
+	// Test the error case
+	actionClient, err = acg.ActionClientFor(ctx, obj)
+	assert.Error(t, err)
+	assert.Nil(t, actionClient)
+}