fix: address panic in response handling and better handling for ModelStatus api method (#1895)

* fix: address panic in secret backend results

* fix: better error handling for ModelStatus api client method

Author: kian99

internal/jujuapi/types.go

@@ -325,7 +325,7 @@ func toFullModelInfo(modelInfo jujuclient.ModelInfo) jujuparams.ModelInfo {
 
 	var secretBackendResults []jujuparams.SecretBackendResult
 	for _, sb := range modelInfo.SecretBackends {
-		secretBackendResults = append(secretBackendResults, jujuparams.SecretBackendResult{
+		res := jujuparams.SecretBackendResult{
 			Result: jujuparams.SecretBackend{
 				Name:                sb.Result.Name,
 				BackendType:         sb.Result.BackendType,
@@ -336,12 +336,15 @@ func toFullModelInfo(modelInfo jujuclient.ModelInfo) jujuparams.ModelInfo {
 			NumSecrets: sb.NumSecrets,
 			Status:     sb.Status,
 			Message:    sb.Message,
-			Error: &jujuparams.Error{
+		}
+		if sb.Error != nil {
+			res.Error = &jujuparams.Error{
 				Message: sb.Error.Error(),
 				Code:    string(errors.ErrorCode(sb.Error)),
 				Info:    errors.ErrorInfo(sb.Error),
-			},
-		})
+			}
+		}
+		secretBackendResults = append(secretBackendResults, res)
 	}
 	modelInfoParams.SecretBackends = secretBackendResults
 

internal/jujuapi/types_test.go

@@ -363,3 +363,69 @@ func TestToFullModelInfo(t *testing.T) {
 		},
 	}})
 }
+
+func TestToFullModelInfoNilSecretBackendError(t *testing.T) {
+	c := qt.New(t)
+
+	modelInfo := jujuclient.ModelInfo{
+		ModelInfo: base.ModelInfo{
+			Name:            "test-model",
+			Cloud:           "aws",
+			CloudCredential: "aws/alice@canonical.com/main-cred",
+			Owner:           "alice@canonical.com",
+		},
+		SecretBackends: []jujuclient.SecretBackendResult{{
+			Result: jujuclient.SecretBackend{
+				Name:        "vault",
+				BackendType: "vault",
+			},
+			ID:         "backend-1",
+			NumSecrets: 7,
+			Status:     "available",
+			Message:    "ready",
+			Error:      nil,
+		}},
+	}
+
+	got := toFullModelInfo(modelInfo)
+
+	c.Assert(got.SecretBackends, qt.DeepEquals, []jujuparams.SecretBackendResult{{
+		Result: jujuparams.SecretBackend{
+			Name:        "vault",
+			BackendType: "vault",
+		},
+		ID:         "backend-1",
+		NumSecrets: 7,
+		Status:     "available",
+		Message:    "ready",
+		Error:      nil,
+	}})
+}
+
+func TestToFullModelInfoNonNilSecretBackendError(t *testing.T) {
+	c := qt.New(t)
+
+	modelInfo := jujuclient.ModelInfo{
+		ModelInfo: base.ModelInfo{
+			Name:            "test-model",
+			Cloud:           "aws",
+			CloudCredential: "aws/alice@canonical.com/main-cred",
+			Owner:           "alice@canonical.com",
+		},
+		SecretBackends: []jujuclient.SecretBackendResult{{
+			Result: jujuclient.SecretBackend{
+				Name:        "vault",
+				BackendType: "vault",
+			},
+			Error: errors.E("an error", errors.CodeNotFound, map[string]any{"detail": "not found"}),
+		}},
+	}
+
+	got := toFullModelInfo(modelInfo)
+
+	c.Assert(got.SecretBackends, qt.HasLen, 1)
+	c.Assert(got.SecretBackends[0].Error, qt.IsNotNil)
+	c.Assert(got.SecretBackends[0].Error.Error(), qt.Equals, "an error")
+	c.Assert(got.SecretBackends[0].Error.Code, qt.Equals, string(errors.CodeNotFound))
+	c.Assert(got.SecretBackends[0].Error.Info, qt.DeepEquals, map[string]any{"detail": "not found"})
+}

internal/jujuclient/modelmanager.go

@@ -142,6 +142,9 @@ func (c Connection) ModelStatus(ctx context.Context, modelTag names.ModelTag) (b
 	if len(statuses) == 0 {
 		return base.ModelStatus{}, errors.E("no status returned for model")
 	}
+	if statuses[0].Error != nil {
+		return base.ModelStatus{}, statuses[0].Error
+	}
 	return statuses[0], nil
 }
 

testing/dial_test.go

@@ -8,10 +8,12 @@ import (
 	"testing"
 
 	qt "github.com/frankban/quicktest"
+	"github.com/google/uuid"
 	"github.com/juju/names/v5"
 	"github.com/juju/version/v2"
 
 	"github.com/canonical/jimm/v3/internal/dbmodel"
+	"github.com/canonical/jimm/v3/internal/errors"
 	"github.com/canonical/jimm/v3/internal/jujuclient"
 	"github.com/canonical/jimm/v3/internal/testutils/jimmtest"
 )
@@ -82,6 +84,29 @@ func TestDialWithJWT(t *testing.T) {
 	c.Check(addrs, qt.ContentEquals, config.Addrs)
 }
 
+func TestDialModelStatusMissingModel(t *testing.T) {
+	c := qt.New(t)
+	s := jimmtest.SetupJimmWithControllers(c)
+
+	name, config := s.GetOneControllerConfig(c)
+	ctl := dbmodel.Controller{
+		UUID:          config.UUID,
+		Name:          name,
+		CACertificate: config.CACert,
+		PublicAddress: config.Addrs[0],
+		TLSHostname:   "juju-apiserver",
+	}
+
+	api, err := s.JIMM.Dialer.Dial(context.Background(), &ctl, names.ModelTag{}, nil, nil)
+	c.Assert(err, qt.Equals, nil)
+	defer api.Close()
+
+	_, err = api.ModelStatus(context.Background(), names.NewModelTag(uuid.NewString()))
+	c.Assert(err, qt.Not(qt.IsNil))
+	c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeNotFound)
+	c.Check(err, qt.ErrorMatches, `model .* not found.*`)
+}
+
 // TestConnectStreams tests the ConnectStream and ConnectControllerStream methods
 // of our Juju dialer. It verifies that we can connect to valid endpoints
 // on a Juju controller, and that invalid endpoints return errors.