diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 868ad4c95..7bd734e10 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -25,6 +25,7 @@ jobs: build_test: name: Build and Test runs-on: ubuntu-22.04 + timeout-minutes: 45 steps: - uses: actions/checkout@v4 with: diff --git a/api/params/params.go b/api/params/params.go index 2666833e5..62ce4ed87 100644 --- a/api/params/params.go +++ b/api/params/params.go @@ -44,6 +44,9 @@ type AddControllerRequest struct { // themselves are migrated. PublicAddress string `json:"public-address,omitempty"` + // TLSHostname is the hostname used for TLS verification. + TLSHostname string `json:"tls-hostname,omitempty"` + // APIAddresses contains the currently known API addresses for the // controller. APIAddresses []string `json:"api-addresses,omitempty"` @@ -347,8 +350,9 @@ type ListRelationshipTuplesRequest struct { // ListRelationshipTuplesResponse holds the response of the ListRelationshipTuples method. type ListRelationshipTuplesResponse struct { - Tuples []RelationshipTuple `json:"tuples,omitempty"` - ContinuationToken string `json:"continuation_token,omitempty"` + Tuples []RelationshipTuple `json:"tuples,omitempty" yaml:"tuples,omitempty"` + Errors []string `json:"errors,omitempty" yaml:"errors,omitempty"` + ContinuationToken string `json:"continuation_token,omitempty" yaml:"continuation_token,omitempty"` } // CrossModelQueryRequest holds the parameters to perform a cross model query against diff --git a/charms/jimm/config.yaml b/charms/jimm/config.yaml index b03a8a42c..c4aa5439b 100644 --- a/charms/jimm/config.yaml +++ b/charms/jimm/config.yaml @@ -84,9 +84,3 @@ options: description: | The max age for the session cookies in seconds, on subsequent logins, the session instance extended by this amount. - final-redirect-url: - type: string - default: "" - description: | - The final redirect URL for JIMM to redirect to when completing a browser based - login. This should be your dashboard. diff --git a/charms/jimm/src/charm.py b/charms/jimm/src/charm.py index ea56d7ac4..6635c5af9 100755 --- a/charms/jimm/src/charm.py +++ b/charms/jimm/src/charm.py @@ -152,7 +152,6 @@ def _on_config_changed(self, _): "session_expiry_duration": self.config.get("session-expiry-duration"), "secure_session_cookies": self.config.get("secure-session-cookies"), "session_cookie_max_age": self.config.get("session-cookie-max-age"), - "final_redirect_url": self.config.get("final-redirect-url"), } self.oauth.update_client_config(client_config=self._oauth_client_config) diff --git a/charms/jimm/templates/jimm.env b/charms/jimm/templates/jimm.env index 714006d03..3798f86bb 100644 --- a/charms/jimm/templates/jimm.env +++ b/charms/jimm/templates/jimm.env @@ -1,6 +1,7 @@ JIMM_ADMINS={{admins}} {% if dashboard_location %} JIMM_DASHBOARD_LOCATION={{dashboard_location}} +JIMM_DASHBOARD_FINAL_REDIRECT_URL={{dashboard_location}} {% endif %} {% if dns_name %} JIMM_DNS_NAME={{dns_name}} @@ -20,4 +21,3 @@ JIMM_MACAROON_EXPIRY_DURATION={{macaroon_expiry_duration}} JIMM_ACCESS_TOKEN_EXPIRY_DURATION={{session_expiry_duration}} JIMM_SECURE_SESSION_COOKIES={{secure_session_cookies}} JIMM_SESSION_COOKIE_MAX_AGE={{session_cookie_max_age}} -JIMM_DASHBOARD_FINAL_REDIRECT_URL={{final_redirect_url}} diff --git a/charms/jimm/tests/test_charm.py b/charms/jimm/tests/test_charm.py index d2de0c56f..7cc7bf58e 100644 --- a/charms/jimm/tests/test_charm.py +++ b/charms/jimm/tests/test_charm.py @@ -172,40 +172,25 @@ def test_config_changed(self): "macaroon-expiry-duration": "48h", "secure-session-cookies": True, "session-cookie-max-age": 86400, - "final-redirect-url": "", } ) self.assertTrue(os.path.exists(config_file)) with open(config_file) as f: - lines = f.readlines() + lines = [line.strip() for line in f.readlines()] os.unlink(config_file) self.assertEqual(len(lines), 19) - self.assertEqual(lines[0].strip(), "JIMM_ADMINS=user1 user2 group1") - self.assertEqual( - lines[2].strip(), - "JIMM_DASHBOARD_LOCATION=https://jaas.ai/models", - ) - self.assertEqual(lines[5].strip(), "JIMM_DNS_NAME=" + "jimm.example.com") - self.assertEqual(lines[7].strip(), "JIMM_LOG_LEVEL=debug") - self.assertEqual(lines[8].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa") - self.assertEqual( - lines[9].strip(), - "BAKERY_PRIVATE_KEY=ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=", - ) - self.assertEqual( - lines[10].strip(), - "BAKERY_PUBLIC_KEY=izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=", - ) - self.assertEqual( - lines[11].strip(), - "JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS=10", - ) - self.assertEqual( - lines[12].strip(), - "JIMM_JWT_EXPIRY=10m", - ) - self.assertEqual(lines[14].strip(), "JIMM_MACAROON_EXPIRY_DURATION=48h") - self.assertEqual(lines[15].strip(), "JIMM_ACCESS_TOKEN_EXPIRY_DURATION=6h") + self.assertIn("JIMM_ADMINS=user1 user2 group1", lines) + self.assertIn("JIMM_DASHBOARD_LOCATION=https://jaas.ai/models", lines) + self.assertIn("JIMM_DASHBOARD_FINAL_REDIRECT_URL=https://jaas.ai/models", lines) + self.assertIn("JIMM_DNS_NAME=jimm.example.com", lines) + self.assertIn("JIMM_LOG_LEVEL=debug", lines) + self.assertIn("JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa", lines) + self.assertIn("BAKERY_PRIVATE_KEY=ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=", lines) + self.assertIn("BAKERY_PUBLIC_KEY=izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=", lines) + self.assertIn("JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS=10", lines) + self.assertIn("JIMM_JWT_EXPIRY=10m", lines) + self.assertIn("JIMM_MACAROON_EXPIRY_DURATION=48h", lines) + self.assertIn("JIMM_ACCESS_TOKEN_EXPIRY_DURATION=6h", lines) def test_config_changed_redirect_to_dashboard(self): config_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm.env") @@ -222,39 +207,24 @@ def test_config_changed_redirect_to_dashboard(self): "macaroon-expiry-duration": "48h", "secure-session-cookies": True, "session-cookie-max-age": 86400, - "final-redirect-url": "", } ) self.assertTrue(os.path.exists(config_file)) with open(config_file) as f: - lines = f.readlines() + lines = [line.strip() for line in f.readlines()] os.unlink(config_file) self.assertEqual(len(lines), 19) - self.assertEqual(lines[0].strip(), "JIMM_ADMINS=user1 user2 group1") - self.assertEqual( - lines[2].strip(), - "JIMM_DASHBOARD_LOCATION=https://test.jaas.ai/models", - ) - self.assertEqual(lines[5].strip(), "JIMM_DNS_NAME=" + "jimm.example.com") - self.assertEqual(lines[7].strip(), "JIMM_LOG_LEVEL=debug") - self.assertEqual(lines[8].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa") - self.assertEqual( - lines[9].strip(), - "BAKERY_PRIVATE_KEY=ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=", - ) - self.assertEqual( - lines[10].strip(), - "BAKERY_PUBLIC_KEY=izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=", - ) - self.assertEqual( - lines[11].strip(), - "JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS=10", - ) - self.assertEqual( - lines[12].strip(), - "JIMM_JWT_EXPIRY=5m", - ) - self.assertEqual(lines[14].strip(), "JIMM_MACAROON_EXPIRY_DURATION=48h") + self.assertIn("JIMM_ADMINS=user1 user2 group1", lines) + self.assertIn("JIMM_DASHBOARD_LOCATION=https://test.jaas.ai/models", lines) + self.assertIn("JIMM_DASHBOARD_FINAL_REDIRECT_URL=https://test.jaas.ai/models", lines) + self.assertIn("JIMM_DNS_NAME=" + "jimm.example.com", lines) + self.assertIn("JIMM_LOG_LEVEL=debug", lines) + self.assertIn("JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa", lines) + self.assertIn("BAKERY_PRIVATE_KEY=ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=", lines) + self.assertIn("BAKERY_PUBLIC_KEY=izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=", lines) + self.assertIn("JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS=10", lines) + self.assertIn("JIMM_JWT_EXPIRY=5m", lines) + self.assertIn("JIMM_MACAROON_EXPIRY_DURATION=48h", lines) def test_config_changed_ready(self): config_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm.env") @@ -270,51 +240,35 @@ def test_config_changed_ready(self): "macaroon-expiry-duration": "48h", "secure-session-cookies": True, "session-cookie-max-age": 86400, - "final-redirect-url": "", } ) self.assertTrue(os.path.exists(config_file)) with open(config_file) as f: - lines = f.readlines() + lines = [line.strip() for line in f.readlines()] os.unlink(config_file) self.assertEqual(len(lines), 17) - self.assertEqual(lines[0].strip(), "JIMM_ADMINS=user1 user2 group1") - self.assertEqual( - lines[2].strip(), - "JIMM_DASHBOARD_LOCATION=https://jaas.ai/models", - ) - self.assertEqual(lines[5].strip(), "JIMM_LOG_LEVEL=info") - self.assertEqual(lines[6].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa") - self.assertEqual( - lines[7].strip(), - "BAKERY_PRIVATE_KEY=ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=", - ) - self.assertEqual( - lines[8].strip(), - "BAKERY_PUBLIC_KEY=izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=", - ) - self.assertEqual( - lines[9].strip(), - "JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS=10", - ) - self.assertEqual( - lines[10].strip(), - "JIMM_JWT_EXPIRY=5m", - ) - self.assertEqual(lines[12].strip(), "JIMM_MACAROON_EXPIRY_DURATION=48h") + self.assertIn("JIMM_ADMINS=user1 user2 group1", lines) + self.assertIn("JIMM_DASHBOARD_LOCATION=https://jaas.ai/models", lines) + self.assertIn("JIMM_LOG_LEVEL=info", lines) + self.assertIn("JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa", lines) + self.assertIn("BAKERY_PRIVATE_KEY=ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=", lines) + self.assertIn("BAKERY_PUBLIC_KEY=izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=", lines) + self.assertIn("JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS=10", lines) + self.assertIn("JIMM_JWT_EXPIRY=5m", lines) + self.assertIn("JIMM_MACAROON_EXPIRY_DURATION=48h", lines) def test_leader_elected(self): leader_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm-leader.env") self.harness.charm.on.leader_elected.emit() with open(leader_file) as f: - lines = f.readlines() - self.assertEqual(lines[0].strip(), "JIMM_WATCH_CONTROLLERS=") - self.assertEqual(lines[1].strip(), "JIMM_ENABLE_JWKS_ROTATOR=") + lines = [line.strip() for line in f.readlines()] + self.assertIn("JIMM_WATCH_CONTROLLERS=", lines) + self.assertIn("JIMM_ENABLE_JWKS_ROTATOR=", lines) self.harness.set_leader(True) with open(leader_file) as f: - lines = f.readlines() - self.assertEqual(lines[0].strip(), "JIMM_WATCH_CONTROLLERS=1") - self.assertEqual(lines[1].strip(), "JIMM_ENABLE_JWKS_ROTATOR=1") + lines = [line.strip() for line in f.readlines()] + self.assertIn("JIMM_WATCH_CONTROLLERS=1", lines) + self.assertIn("JIMM_ENABLE_JWKS_ROTATOR=1", lines) def test_leader_elected_ready(self): leader_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm-leader.env") @@ -324,14 +278,14 @@ def test_leader_elected_ready(self): f.write("test") self.harness.charm.on.leader_elected.emit() with open(leader_file) as f: - lines = f.readlines() - self.assertEqual(lines[0].strip(), "JIMM_WATCH_CONTROLLERS=") + lines = [line.strip() for line in f.readlines()] + self.assertIn("JIMM_WATCH_CONTROLLERS=", lines) self.harness.set_leader(True) self.add_oauth_relation() self.add_openfga_relation() with open(leader_file) as f: - lines = f.readlines() - self.assertEqual(lines[0].strip(), "JIMM_WATCH_CONTROLLERS=1") + lines = [line.strip() for line in f.readlines()] + self.assertIn("JIMM_WATCH_CONTROLLERS=1", lines) self.harness.charm._systemctl.assert_has_calls( ( call("is-enabled", self.harness.charm.service), @@ -353,14 +307,14 @@ def test_database_relation_changed(self): }, ) with open(db_file) as f: - lines = f.readlines() + lines = [line.strip() for line in f.readlines()] self.assertEqual(len(lines), 1) - self.assertEqual(lines[0].strip(), "JIMM_DSN=postgresql://some-username:some-password@some.database.host/jimm") + self.assertIn("JIMM_DSN=postgresql://some-username:some-password@some.database.host/jimm", lines) self.harness.update_relation_data(id, "postgresql", {}) with open(db_file) as f: - lines = f.readlines() + lines = [line.strip() for line in f.readlines()] self.assertEqual(len(lines), 1) - self.assertEqual(lines[0].strip(), "JIMM_DSN=postgresql://some-username:some-password@some.database.host/jimm") + self.assertIn("JIMM_DSN=postgresql://some-username:some-password@some.database.host/jimm", lines) def test_database_relation_changed_ready(self): db_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm-db.env") @@ -381,14 +335,14 @@ def test_database_relation_changed_ready(self): }, ) with open(db_file) as f: - lines = f.readlines() + lines = [line.strip() for line in f.readlines()] self.assertEqual(len(lines), 1) - self.assertEqual(lines[0].strip(), "JIMM_DSN=postgresql://some-username:some-password@some.database.host/jimm") + self.assertIn("JIMM_DSN=postgresql://some-username:some-password@some.database.host/jimm", lines) self.harness.update_relation_data(id, "postgresql", {}) with open(db_file) as f: - lines = f.readlines() + lines = [line.strip() for line in f.readlines()] self.assertEqual(len(lines), 1) - self.assertEqual(lines[0].strip(), "JIMM_DSN=postgresql://some-username:some-password@some.database.host/jimm") + self.assertIn("JIMM_DSN=postgresql://some-username:some-password@some.database.host/jimm", lines) self.harness.charm._systemctl.assert_has_calls( ( call("is-enabled", self.harness.charm.service), @@ -447,14 +401,14 @@ def test_vault_relation_changed(self): {"data": {"role_id": "test-role-id", "secret_id": "test-secret"}}, ) with open(self.harness.charm._env_filename("vault")) as f: - lines = f.readlines() - self.assertEqual(lines[0].strip(), "VAULT_ADDR=http://vault:8200") - self.assertEqual(lines[1].strip(), "VAULT_PATH=charm-jimm-creds") + lines = [line.strip() for line in f.readlines()] + self.assertIn("VAULT_ADDR=http://vault:8200", lines) + self.assertIn("VAULT_PATH=charm-jimm-creds", lines) self.assertEqual( lines[2].strip(), "VAULT_SECRET_FILE={}".format(self.harness.charm._vault_secret_filename), ) - self.assertEqual(lines[3].strip(), "VAULT_AUTH_PATH=/auth/approle/login") + self.assertIn("VAULT_AUTH_PATH=/auth/approle/login", lines) def test_stop(self): self.harness.charm.on.stop.emit() @@ -540,13 +494,13 @@ def test_openfga_relation_changed(self): self.add_openfga_relation() with open(self.harness.charm._env_filename("openfga")) as f: - lines = f.readlines() + lines = [line.strip() for line in f.readlines()] - self.assertEqual(lines[0].strip(), "OPENFGA_HOST=openfga.localhost") - self.assertEqual(lines[1].strip(), "OPENFGA_PORT=8080") - self.assertEqual(lines[2].strip(), "OPENFGA_SCHEME=http") - self.assertEqual(lines[3].strip(), "OPENFGA_STORE=fake-store-id") - self.assertEqual(lines[4].strip(), "OPENFGA_TOKEN=fake-token") + self.assertIn("OPENFGA_HOST=openfga.localhost", lines) + self.assertIn("OPENFGA_PORT=8080", lines) + self.assertIn("OPENFGA_SCHEME=http", lines) + self.assertIn("OPENFGA_STORE=fake-store-id", lines) + self.assertIn("OPENFGA_TOKEN=fake-token", lines) def test_insecure_secret_storage(self): """Test that the flag for insecure secret storage is only generated when explicitly requested.""" @@ -563,14 +517,14 @@ def test_insecure_secret_storage(self): ) self.assertTrue(os.path.exists(config_file)) with open(config_file) as f: - lines = f.readlines() + lines = [line.strip() for line in f.readlines()] os.unlink(config_file) self.assertEqual(len(lines), 19) self.assertEqual(len([match for match in lines if "INSECURE_SECRET_STORAGE" in match]), 0) self.harness.update_config({"postgres-secret-storage": True}) self.assertTrue(os.path.exists(config_file)) with open(config_file) as f: - lines = f.readlines() + lines = [line.strip() for line in f.readlines()] os.unlink(config_file) self.assertEqual(len(lines), 21) self.assertEqual(len([match for match in lines if "INSECURE_SECRET_STORAGE" in match]), 1) @@ -580,11 +534,11 @@ def test_oauth_relation_changed(self): self.add_oauth_relation() with open(self.harness.charm._env_filename("oauth")) as f: - lines = f.readlines() - self.assertEqual(lines[0].strip(), "JIMM_OAUTH_ISSUER_URL=https://example.oidc.com") - self.assertEqual(lines[1].strip(), "JIMM_OAUTH_CLIENT_ID=jimm_client_id") - self.assertEqual(lines[2].strip(), "JIMM_OAUTH_CLIENT_SECRET=test-secret") - self.assertEqual(lines[3].strip(), "JIMM_OAUTH_SCOPES=openid profile email phone") + lines = [line.strip() for line in f.readlines()] + self.assertIn("JIMM_OAUTH_ISSUER_URL=https://example.oidc.com", lines) + self.assertIn("JIMM_OAUTH_CLIENT_ID=jimm_client_id", lines) + self.assertIn("JIMM_OAUTH_CLIENT_SECRET=test-secret", lines) + self.assertIn("JIMM_OAUTH_SCOPES=openid profile email phone", lines) class VersionHTTPRequestHandler(BaseHTTPRequestHandler): diff --git a/cmd/jimmctl/cmd/controllerinfo.go b/cmd/jimmctl/cmd/controllerinfo.go index 30d6fed96..4d15d74ff 100644 --- a/cmd/jimmctl/cmd/controllerinfo.go +++ b/cmd/jimmctl/cmd/controllerinfo.go @@ -52,6 +52,7 @@ type controllerInfoCommand struct { publicAddress string file cmd.FileVar local bool + tlsHostname string } func (c *controllerInfoCommand) Info() *cmd.Info { @@ -66,6 +67,7 @@ func (c *controllerInfoCommand) Info() *cmd.Info { func (c *controllerInfoCommand) SetFlags(f *gnuflag.FlagSet) { c.CommandBase.SetFlags(f) f.BoolVar(&c.local, "local", false, "If local flag is specified, then the local API address and CA cert of the controller will be used.") + f.StringVar(&c.tlsHostname, "tls-hostname", "", "Specify the hostname for TLS verfiication.") } // Init implements the cmd.Command interface. @@ -80,6 +82,9 @@ func (c *controllerInfoCommand) Init(args []string) error { if len(args) > 3 { return errors.New("too many args") } + if c.local && len(c.publicAddress) > 0 { + return errors.New("cannot set both public address and local flag") + } return nil } @@ -103,6 +108,7 @@ func (c *controllerInfoCommand) Run(ctxt *cmd.Context) error { Password: accountDetails.Password, } + info.TLSHostname = c.tlsHostname info.PublicAddress = c.publicAddress if c.local { info.PublicAddress = controller.APIEndpoints[0] @@ -111,9 +117,6 @@ func (c *controllerInfoCommand) Run(ctxt *cmd.Context) error { if info.PublicAddress == "" { return errors.New("public address must be set") } - if c.local && len(c.publicAddress) > 0 { - return errors.New("please do not set both the address argument and the local flag") - } data, err := yaml.Marshal(info) if err != nil { diff --git a/cmd/jimmctl/cmd/controllerinfo_test.go b/cmd/jimmctl/cmd/controllerinfo_test.go index e7d46ebc9..32c98d428 100644 --- a/cmd/jimmctl/cmd/controllerinfo_test.go +++ b/cmd/jimmctl/cmd/controllerinfo_test.go @@ -244,5 +244,62 @@ func (s *controllerInfoSuite) TestControllerInfoCannotProvideAddrAndLocalFlag(c fname := path.Join(dir, "test.yaml") _, err = cmdtesting.RunCommand(c, cmd.NewControllerInfoCommandForTesting(store), "controller-1", fname, "myaddress", "--local") - c.Assert(err, gc.ErrorMatches, "please do not set both the address argument and the local flag") + c.Assert(err, gc.ErrorMatches, "cannot set both public address and local flag") +} + +func (s *controllerInfoSuite) TestControllerInfoWithTlsFlag(c *gc.C) { + store := s.ClientStore() + store.Controllers["controller-1"] = jujuclient.ControllerDetails{ + ControllerUUID: "982b16d9-a945-4762-b684-fd4fd885aa11", + APIEndpoints: []string{"127.0.0.1:17070"}, + PublicDNSName: "controller1.example.com", + CACert: `-----BEGIN CERTIFICATE----- + MIID/jCCAmagAwIBAgIVANxsMrzsXrdpjjUoxWQm1RCkmWcqMA0GCSqGSIb3DQEB + CwUAMCYxDTALBgNVBAoTBEp1anUxFTATBgNVBAMTDGp1anUgdGVzdGluZzAeFw0y + MDA0MDgwNTI3NTBaFw0zMDA0MDgwNTMyNTBaMCYxDTALBgNVBAoTBEp1anUxFTAT + BgNVBAMTDGp1anUgdGVzdGluZzCCAaIwDQYJKoZIhvcNAQEBBQADggGPADCCAYoC + ggGBAOW4k2bmXXU3tJ8H5AsGkp8ENLJXzU4SCOCB+X0jPQRVpFtywBVD96z+l+qW + ndGLIg5zMQTtZm71CaOw+8Sl03XU0f28Xrjf+FZCAPID1c7NBttUShbu84euFoCS + C8yobj6JzLz7QswvkshYQ7JEZ88UXtVHqg6MGYFdu+cX/dE1jC7aHg9bus/P6bFH + PVFcHVVxNbLy98Id1iB7i0s97H17nu9O7ZKMrAQAX6dfAELAFQVicdN3WpfwNXEj + M2KIrqttpM8s6/57mi9UJFYGdAEDNkJr/dI506VdGLpiqTFhQK6ztfDfY08QbWk6 + iJn8vzWvNW8WthmBtEDpv+DL+a5SJSLSAIZn9sbuBBpiX+csZb66fYhKFFIUrIa5 + lrjw8yiHJ4kgsEZJSYaAn7guqmOv8clvy1E2JjsOfGycest6+1/mNdMRFgrMxdzD + 0M2yZ96zrdfF/QXpi7Hk7jFLzimuujNUpKFv7k+XObQFxeXnoFrYVkj3YT8hhYF0 + mGRkAwIDAQABoyMwITAOBgNVHQ8BAf8EBAMCAqQwDwYDVR0TAQH/BAUwAwEB/zAN + BgkqhkiG9w0BAQsFAAOCAYEAd7GrziPRmjoK3HDF10S+5NgoKYvkOuk2jDap2Qaq + ZFjDvrDA2tr6U0FGY+Hz+QfvtgT+YpJB5IvABvSXdq37llwKGsiSOZSrpHyTsOB0 + VcZAF3GMk1nHYMr0o1xRV2gm/ax+vUEStj0k2gTs/p57uhKcCUXR0p3PWXKcRj9a + nVf5bdVkt6ghGs7/uEvj303raUFSf5dJ4C9RTgBK2E9/wlBYNyj5vcsshNpz8kt6 + RuARM6Boq2EwKkpRlbvImDM8ZJJLwtpijYrx3egfOSEA7Wfwgwn+B359XcosOee5 + n5BC62EjaP85cM9HCtp2DwKjNSosxja723qZPY6Z0Y7IVn3JVjgC2kWP6GViwb+v + l9uwx9ASHPz9ilh6gpjgIifOKZYCaBSS9g8VxHpO5Izxj4vi4AX5cebDg3SzDVik + hZuWHpuOT120okoutwuUSU9448cXLGZfoCZjjdMKXmOj8EEec1diDP4mhegYGezD + LQRNNlaY2ajLt0paowf/Xxb8 + -----END CERTIFICATE-----`, + } + store.Accounts["controller-1"] = jujuclient.AccountDetails{ + User: "test-user", + Password: "super-secret-password", + } + dir, err := os.MkdirTemp("", "controller-info-test") + c.Assert(err, gc.Equals, nil) + defer os.RemoveAll(dir) + + fname := path.Join(dir, "test.yaml") + + _, err = cmdtesting.RunCommand(c, cmd.NewControllerInfoCommandForTesting(store), "controller-1", fname, "myaddress", "--tls-hostname", "foo") + c.Assert(err, gc.IsNil) + + data, err := os.ReadFile(fname) + c.Assert(err, gc.IsNil) + c.Assert(string(data), gc.Matches, `api-addresses: +- 127.0.0.1:17070 +name: controller-1 +password: super-secret-password +public-address: myaddress +tls-hostname: foo +username: test-user +uuid: 982b16d9-a945-4762-b684-fd4fd885aa11 +`) } diff --git a/cmd/jimmctl/cmd/migratemodel.go b/cmd/jimmctl/cmd/migratemodel.go index f17228d62..06770b63f 100644 --- a/cmd/jimmctl/cmd/migratemodel.go +++ b/cmd/jimmctl/cmd/migratemodel.go @@ -20,16 +20,14 @@ import ( var migrateModelCommandDoc = ` The migrate command migrates a model(s) to a new controller. Specify - a model-tag to migrate and the destination controller name. - A model-tag is of the form "model-" while a controller name is - simply the name of the controller. + a model-uuid to migrate and the destination controller name. Note that multiple models can be targeted for migration by supplying - multiple model tags. + multiple model uuids. Example: - jimmctl migrate - jimmctl migrate + jimmctl migrate + jimmctl migrate ` // NewMigrateModelCommand returns a command to migrate models. @@ -72,18 +70,19 @@ func (c *migrateModelCommand) SetFlags(f *gnuflag.FlagSet) { // Init implements the cmd.Command interface. func (c *migrateModelCommand) Init(args []string) error { if len(args) < 2 { - return errors.E("Missing controller and model tag arguments") + return errors.E("Missing controller name and model uuid arguments") } for i, arg := range args { if i == 0 { c.targetController = arg continue } - _, err := names.ParseModelTag(arg) + mt := names.NewModelTag(arg) + _, err := names.ParseModelTag(mt.String()) if err != nil { - return errors.E(err, fmt.Sprintf("%s is not a valid model tag", arg)) + return errors.E(err, fmt.Sprintf("%s is not a valid model uuid", arg)) } - c.modelTags = append(c.modelTags, arg) + c.modelTags = append(c.modelTags, mt.String()) } return nil } diff --git a/cmd/jimmctl/cmd/migratemodel_test.go b/cmd/jimmctl/cmd/migratemodel_test.go index 3fc953a46..f0ef7cdb3 100644 --- a/cmd/jimmctl/cmd/migratemodel_test.go +++ b/cmd/jimmctl/cmd/migratemodel_test.go @@ -48,7 +48,7 @@ func (s *migrateModelSuite) TestMigrateModelCommandSuperuser(c *gc.C) { // alice is superuser bClient := jimmtest.NewUserSessionLogin(c, "alice") - context, err := cmdtesting.RunCommand(c, cmd.NewMigrateModelCommandForTesting(s.ClientStore(), bClient), "controller-1", mt.String(), mt2.String()) + context, err := cmdtesting.RunCommand(c, cmd.NewMigrateModelCommandForTesting(s.ClientStore(), bClient), "controller-1", mt.Id(), mt2.Id()) c.Assert(err, gc.IsNil) c.Assert(cmdtesting.Stdout(context), gc.Matches, migrationResultRegex) } @@ -62,12 +62,12 @@ func (s *migrateModelSuite) TestMigrateModelCommandFailsWithInvalidModelTag(c *g // alice is superuser bClient := jimmtest.NewUserSessionLogin(c, "alice") - _, err := cmdtesting.RunCommand(c, cmd.NewMigrateModelCommandForTesting(s.ClientStore(), bClient), "controller-1", "model-001", "model-002") - c.Assert(err, gc.ErrorMatches, ".* is not a valid model tag") + _, err := cmdtesting.RunCommand(c, cmd.NewMigrateModelCommandForTesting(s.ClientStore(), bClient), "controller-1", "001", "002") + c.Assert(err, gc.ErrorMatches, ".* is not a valid model uuid") } func (s *migrateModelSuite) TestMigrateModelCommandFailsWithMissingArgs(c *gc.C) { bClient := jimmtest.NewUserSessionLogin(c, "alice") _, err := cmdtesting.RunCommand(c, cmd.NewMigrateModelCommandForTesting(s.ClientStore(), bClient), "myController") - c.Assert(err, gc.ErrorMatches, "Missing controller and model tag arguments") + c.Assert(err, gc.ErrorMatches, "Missing controller name and model uuid arguments") } diff --git a/cmd/jimmctl/cmd/relation.go b/cmd/jimmctl/cmd/relation.go index 2bb087b99..03fb25e0e 100644 --- a/cmd/jimmctl/cmd/relation.go +++ b/cmd/jimmctl/cmd/relation.go @@ -549,7 +549,9 @@ func (c *listRelationsCommand) Run(ctxt *cmd.Context) error { return errors.E(err) } - err = c.out.Write(ctxt, result.Tuples) + // Ensure continutation token is empty so that we don't print it. + result.ContinuationToken = "" + err = c.out.Write(ctxt, result) if err != nil { return errors.E(err) } @@ -573,9 +575,9 @@ func fetchRelations(client *api.Client, params apiparams.ListRelationshipTuplesR } func formatRelationsTabular(writer io.Writer, value interface{}) error { - tuples, ok := value.([]apiparams.RelationshipTuple) + resp, ok := value.(*apiparams.ListRelationshipTuplesResponse) if !ok { - return errors.E(fmt.Sprintf("expected value of type %T, got %T", tuples, value)) + return errors.E(fmt.Sprintf("expected value of type %T, got %T", resp, value)) } table := uitable.New() @@ -583,9 +585,17 @@ func formatRelationsTabular(writer io.Writer, value interface{}) error { table.Wrap = true table.AddRow("Object", "Relation", "Target Object") - for _, tuple := range tuples { + for _, tuple := range resp.Tuples { table.AddRow(tuple.Object, tuple.Relation, tuple.TargetObject) } fmt.Fprint(writer, table) + + if len(resp.Errors) != 0 { + fmt.Fprintf(writer, "\n\n") + fmt.Fprintln(writer, "Errors") + for _, msg := range resp.Errors { + fmt.Fprintln(writer, msg) + } + } return nil } diff --git a/cmd/jimmctl/cmd/relation_test.go b/cmd/jimmctl/cmd/relation_test.go index c240b2a23..3337de871 100644 --- a/cmd/jimmctl/cmd/relation_test.go +++ b/cmd/jimmctl/cmd/relation_test.go @@ -18,7 +18,6 @@ import ( "github.com/juju/names/v5" gc "gopkg.in/check.v1" yamlv2 "gopkg.in/yaml.v2" - "gopkg.in/yaml.v3" apiparams "github.com/canonical/jimm/api/params" "github.com/canonical/jimm/cmd/jimmctl/cmd" @@ -383,7 +382,7 @@ func (s *relationSuite) TestListRelations(c *gc.C) { c.Assert(err, gc.IsNil) } - expectedJSONData, err := json.Marshal(append( + expectedData := apiparams.ListRelationshipTuplesResponse{Tuples: append( []apiparams.RelationshipTuple{{ Object: "user-admin", Relation: "administrator", @@ -394,20 +393,11 @@ func (s *relationSuite) TestListRelations(c *gc.C) { TargetObject: "controller-jimm", }}, relations..., - )) + )} + expectedJSONData, err := json.Marshal(expectedData) c.Assert(err, gc.IsNil) - expectedYAMLData, err := yaml.Marshal(append( - []apiparams.RelationshipTuple{{ - Object: "user-admin", - Relation: "administrator", - TargetObject: "controller-jimm", - }, { - Object: "user-alice@canonical.com", - Relation: "administrator", - TargetObject: "controller-jimm", - }}, - relations..., - )) + // Necessary to use yamlv2 to match what Juju does. + expectedYAMLData, err := yamlv2.Marshal(expectedData) c.Assert(err, gc.IsNil) context, err := cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "json") @@ -435,6 +425,75 @@ user-eve@canonical.com administrator applicationoffer-test-controller-1:alice@ ) } +func (s *relationSuite) TestListRelationsWithError(c *gc.C) { + env := initializeEnvironment(c, context.Background(), &s.JIMM.Database, *s.AdminUser) + // alice is superuser + bClient := jimmtest.NewUserSessionLogin(c, "alice") + + _, err := cmdtesting.RunCommand(c, cmd.NewAddGroupCommandForTesting(s.ClientStore(), bClient), "group-1") + c.Assert(err, gc.IsNil) + + relation := apiparams.RelationshipTuple{ + Object: "user-" + env.users[0].Name, + Relation: "member", + TargetObject: "group-group-1", + } + _, err = cmdtesting.RunCommand(c, cmd.NewAddRelationCommandForTesting(s.ClientStore(), bClient), relation.Object, relation.Relation, relation.TargetObject) + c.Assert(err, gc.IsNil) + + ctx := context.Background() + group := &dbmodel.GroupEntry{Name: "group-1"} + err = s.JIMM.DB().GetGroup(ctx, group) + c.Assert(err, gc.IsNil) + err = s.JIMM.DB().RemoveGroup(ctx, group) + c.Assert(err, gc.IsNil) + + expectedData := apiparams.ListRelationshipTuplesResponse{ + Tuples: []apiparams.RelationshipTuple{{ + Object: "user-admin", + Relation: "administrator", + TargetObject: "controller-jimm", + }, { + Object: "user-alice@canonical.com", + Relation: "administrator", + TargetObject: "controller-jimm", + }, { + Object: "user-" + env.users[0].Name, + Relation: "member", + TargetObject: "group:" + group.UUID, + }}, + Errors: []string{ + "failed to parse target: failed to fetch group information: " + group.UUID, + }, + } + expectedJSONData, err := json.Marshal(expectedData) + c.Assert(err, gc.IsNil) + // Necessary to use yamlv2 to match what Juju does. + expectedYAMLData, err := yamlv2.Marshal(expectedData) + c.Assert(err, gc.IsNil) + + context, err := cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "json") + c.Assert(err, gc.IsNil) + c.Assert(strings.TrimRight(cmdtesting.Stdout(context), "\n"), gc.Equals, string(expectedJSONData)) + + context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient)) + c.Assert(err, gc.IsNil) + c.Assert(cmdtesting.Stdout(context), gc.Equals, string(expectedYAMLData)) + + context, err = cmdtesting.RunCommand(c, cmd.NewListRelationsCommandForTesting(s.ClientStore(), bClient), "--format", "tabular") + c.Assert(err, gc.IsNil) + expectedOutput := fmt.Sprintf( + `Object Relation Target Object +user-admin administrator controller-jimm +user-alice@canonical.com administrator controller-jimm +user-alice@canonical.com member group:%s + +Errors +failed to parse target: failed to fetch group information: %s +`, group.UUID, group.UUID) + c.Assert(cmdtesting.Stdout(context), gc.Equals, expectedOutput) +} + // TODO: remove boilerplate of env setup and use initialiseEnvironment func (s *relationSuite) TestCheckRelationViaSuperuser(c *gc.C) { ctx := context.TODO() diff --git a/cmd/jimmsrv/main.go b/cmd/jimmsrv/main.go index 357d3ba57..ecbb7c108 100644 --- a/cmd/jimmsrv/main.go +++ b/cmd/jimmsrv/main.go @@ -201,8 +201,10 @@ func start(ctx context.Context, s *service.Service) error { s.OnShutdown(func() { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() + zapctx.Warn(ctx, "server shutdown triggered") httpsrv.Shutdown(ctx) + jimmsvc.Cleanup() }) s.Go(httpsrv.ListenAndServe) zapctx.Info(ctx, "Successfully started JIMM server") diff --git a/export_test.go b/export_test.go index 1b95f4a68..62274dd2e 100644 --- a/export_test.go +++ b/export_test.go @@ -2,3 +2,8 @@ package jimm var NewOpenFGAClient = newOpenFGAClient + +// GetCleanups export `Service.cleanups` field for testing purposes. +func (s *Service) GetCleanups() []func() error { + return s.cleanups +} diff --git a/go.mod b/go.mod index a4c537469..624301703 100644 --- a/go.mod +++ b/go.mod @@ -104,10 +104,7 @@ require ( github.com/aws/smithy-go v1.19.0 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/bmizerany/pat v0.0.0-20210406213842-e4b6760bdd6f // indirect - github.com/canonical/go-flags v0.0.0-20230403090104-105d09a091b8 // indirect github.com/canonical/lxd v0.0.0-20231214113525-e676fc63c50a // indirect - github.com/canonical/pebble v1.10.2 // indirect - github.com/canonical/x-go v0.0.0-20230522092633-7947a7587f5b // indirect github.com/cenkalti/backoff/v3 v3.2.2 // indirect github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/cjlapao/common-go v0.0.39 // indirect @@ -147,7 +144,6 @@ require ( github.com/google/s2a-go v0.1.7 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect github.com/googleapis/gax-go/v2 v2.12.0 // indirect - github.com/gorilla/mux v1.8.1 // indirect github.com/gorilla/schema v1.2.1 // indirect github.com/gorilla/securecookie v1.1.2 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect @@ -264,7 +260,6 @@ require ( github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pkg/sftp v1.13.6 // indirect - github.com/pkg/term v1.1.0 // indirect github.com/pkg/xattr v0.4.9 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_model v0.5.0 // indirect @@ -304,12 +299,10 @@ require ( go.uber.org/multierr v1.11.0 // indirect golang.org/x/crypto v0.22.0 // indirect golang.org/x/exp v0.0.0-20231127185646-65229373498e // indirect - golang.org/x/mod v0.14.0 // indirect golang.org/x/sys v0.19.0 // indirect golang.org/x/term v0.19.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/time v0.5.0 // indirect - golang.org/x/tools v0.16.1 // indirect google.golang.org/api v0.154.0 // indirect google.golang.org/appengine v1.6.8 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20231127180814-3a041ad873d4 // indirect diff --git a/go.sum b/go.sum index 878eda19b..f2e0bce69 100644 --- a/go.sum +++ b/go.sum @@ -145,18 +145,12 @@ github.com/bmizerany/pat v0.0.0-20210406213842-e4b6760bdd6f h1:gOO/tNZMjjvTKZWpY github.com/bmizerany/pat v0.0.0-20210406213842-e4b6760bdd6f/go.mod h1:8rLXio+WjiTceGBHIoTvn60HIbs7Hm7bcHjyrSqYB9c= github.com/canonical/go-dqlite v1.21.0 h1:4gLDdV2GF+vg0yv9Ff+mfZZNQ1JGhnQ3GnS2GeZPHfA= github.com/canonical/go-dqlite v1.21.0/go.mod h1:Uvy943N8R4CFUAs59A1NVaziWY9nJ686lScY7ywurfg= -github.com/canonical/go-flags v0.0.0-20230403090104-105d09a091b8 h1:zGaJEJI9qPVyM+QKFJagiyrM91Ke5S9htoL1D470g6E= -github.com/canonical/go-flags v0.0.0-20230403090104-105d09a091b8/go.mod h1:ZZFeR9K9iGgpwOaLYF9PdT44/+lfSJ9sQz3B+SsGsYU= github.com/canonical/go-service v1.0.0 h1:TF6TsEp04xAoI5pPoWjTYmEwLjbPATSnHEyeJCvzElg= github.com/canonical/go-service v1.0.0/go.mod h1:GzNLXpkGdglL0kjREXoLXj2rB2Qx+EvAGncRDqCENYQ= github.com/canonical/lxd v0.0.0-20231214113525-e676fc63c50a h1:Tfo/MzXK5GeG7gzSHqxGeY/669Mhh5ea43dn1mRDnk8= github.com/canonical/lxd v0.0.0-20231214113525-e676fc63c50a/go.mod h1:UxfHGKFoRjgu1NUA9EFiR++dKvyAiT0h9HT0ffMlzjc= github.com/canonical/ofga v0.10.0 h1:DHXhG/DAXWWQT/I+2jzr4qm0uTIYrILmtMxd6ZqmEzE= github.com/canonical/ofga v0.10.0/go.mod h1:u4Ou8dbIhO7FmVlT7W3rX2roD9AOGz/CqmGh7AdF0Lo= -github.com/canonical/pebble v1.10.2 h1:TG0RYLqH+WEjnxsTB1JbaW0wzeygG0/dPHEEFQKn2JE= -github.com/canonical/pebble v1.10.2/go.mod h1:BXpt85cFqrBgACeVRrTQ7JxZIdnGILv32V7mAfDcGFc= -github.com/canonical/x-go v0.0.0-20230522092633-7947a7587f5b h1:Da2fardddn+JDlVEYtrzBLTtyzoyU3nIS0Cf0GvjmwU= -github.com/canonical/x-go v0.0.0-20230522092633-7947a7587f5b/go.mod h1:upTK9n6rlqITN9rCN69hdreI37dRDFUk2thlGGD5Cg8= github.com/cenkalti/backoff/v3 v3.0.0/go.mod h1:cIeZDE3IrqwwJl6VUwCN6trj1oXrTS4rc0ij+ULvLYs= github.com/cenkalti/backoff/v3 v3.2.2 h1:cfUAAO3yvKMYKPrvhDuHSwQnhZNk/RMHKdZqKTxfm6M= github.com/cenkalti/backoff/v3 v3.2.2/go.mod h1:cIeZDE3IrqwwJl6VUwCN6trj1oXrTS4rc0ij+ULvLYs= @@ -934,8 +928,6 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pkg/sftp v1.10.1/go.mod h1:lYOWFsE0bwd1+KfKJaKeuokY15vzFx25BLbzYYoAxZI= github.com/pkg/sftp v1.13.6 h1:JFZT4XbOU7l77xGSpOdW+pwIMqP044IyjXX6FGyEKFo= github.com/pkg/sftp v1.13.6/go.mod h1:tz1ryNURKu77RL+GuCzmoJYxQczL3wLNNpPWagdg4Qk= -github.com/pkg/term v1.1.0 h1:xIAAdCMh3QIAy+5FrE8Ad8XoDhEU4ufwbaSozViP9kk= -github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw= github.com/pkg/xattr v0.4.9 h1:5883YPCtkSd8LFbs13nXplj9g9tlrwoJRjgpgMu1/fE= github.com/pkg/xattr v0.4.9/go.mod h1:di8WF84zAKk8jzR1UBTEWh9AUlIZZ7M/JNt8e9B6ktU= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -1186,8 +1178,6 @@ golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/mod v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0= -golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20150829230318-ea47fc708ee3/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180406214816-61147c48b25b/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -1310,7 +1300,6 @@ golang.org/x/sys v0.0.0-20200523222454-059865788121/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200728102440-3e129f6d46b1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200803210538-64077c9b5642/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200905004654-be1d3432aa8f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200909081042-eff7692f9009/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201201145000-ef89a241ccb3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= diff --git a/internal/constants/constants.go b/internal/constants/constants.go deleted file mode 100644 index b30fc897d..000000000 --- a/internal/constants/constants.go +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2023 Canonical Ltd. - -// Package constants contains constants and enums used throughout JIMM. -package constants - -// The constants below can be split out once we have more. - -// ModelLife values specify model life status -type ModelLife int - -// Enumerate all possible migration phases. -const ( - UNKNOWN ModelLife = iota - ALIVE - DEAD - DYING - MIGRATING_INTERNAL - MIGRATING_AWAY -) - -var lifeNames = []string{ - "unknown", - "alive", - "dead", - "dying", - "migrating-internal", - "migrating-away", -} - -// String returns the name of an model life constant. -func (p ModelLife) String() string { - i := int(p) - if i >= 0 && i < len(lifeNames) { - return lifeNames[i] - } - return "unknown" -} - -// Parselife converts a string model life name -// to its constant value. -func ParseModelLife(target string) (ModelLife, bool) { - for p, name := range lifeNames { - if target == name { - return ModelLife(p), true - } - } - return UNKNOWN, false -} diff --git a/internal/db/applicationoffer_test.go b/internal/db/applicationoffer_test.go index 69ce00211..d0c7c6a9e 100644 --- a/internal/db/applicationoffer_test.go +++ b/internal/db/applicationoffer_test.go @@ -10,8 +10,8 @@ import ( qt "github.com/frankban/quicktest" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/juju/juju/state" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/db" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/errors" @@ -83,7 +83,7 @@ func initTestEnvironment(c *qt.C, db *db.Database) testEnvironment { Type: "iaas", IsController: false, DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: sql.NullTime{ diff --git a/internal/db/controller_test.go b/internal/db/controller_test.go index ca0404808..99bdc5eb9 100644 --- a/internal/db/controller_test.go +++ b/internal/db/controller_test.go @@ -4,15 +4,12 @@ package db_test import ( "context" - "database/sql" "testing" - "time" qt "github.com/frankban/quicktest" "github.com/google/go-cmp/cmp/cmpopts" jujuparams "github.com/juju/juju/rpc/params" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/db" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/errors" @@ -72,7 +69,6 @@ func (s *dbSuite) TestGetController(c *qt.C) { controller := dbmodel.Controller{ Name: "test-controller", UUID: "00000000-0000-0000-0000-0000-0000000000001", - Models: []dbmodel.Model{}, CloudName: "test-cloud", } err = s.Database.AddController(context.Background(), &controller) @@ -102,105 +98,6 @@ func (s *dbSuite) TestGetController(c *qt.C) { c.Assert(eError.Code, qt.Equals, errors.CodeNotFound) } -func (s *dbSuite) TestGetControllerWithModels(c *qt.C) { - err := s.Database.Migrate(context.Background(), true) - c.Assert(err, qt.Equals, nil) - - cloud := dbmodel.Cloud{ - Name: "test-cloud", - Type: "test-provider", - Regions: []dbmodel.CloudRegion{{ - Name: "test-region", - }}, - } - c.Assert(s.Database.DB.Create(&cloud).Error, qt.IsNil) - - controller := dbmodel.Controller{ - Name: "test-controller", - UUID: "00000000-0000-0000-0000-0000-0000000000001", - Models: []dbmodel.Model{}, - CloudName: "test-cloud", - CloudRegion: "test-region", - } - u, err := dbmodel.NewIdentity("bob@canonical.com") - c.Assert(err, qt.IsNil) - c.Assert(s.Database.DB.Create(&u).Error, qt.IsNil) - - cred := dbmodel.CloudCredential{ - Name: "test-cred", - Cloud: cloud, - Owner: *u, - AuthType: "empty", - } - c.Assert(s.Database.DB.Create(&cred).Error, qt.IsNil) - - err = s.Database.AddController(context.Background(), &controller) - c.Assert(err, qt.Equals, nil) - - models := []dbmodel.Model{{ - Name: "test-model-1", - UUID: sql.NullString{ - String: "00000001-0000-0000-0000-0000-000000000001", - Valid: true, - }, - Owner: *u, - Controller: controller, - CloudRegion: cloud.Regions[0], - CloudCredential: cred, - Type: "iaas", - IsController: true, - DefaultSeries: "warty", - Life: constants.ALIVE.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now(), - Valid: true, - }, - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, - }, { - Name: "test-model-2", - UUID: sql.NullString{ - String: "00000001-0000-0000-0000-0000-000000000002", - Valid: true, - }, - Owner: *u, - Controller: controller, - CloudRegion: cloud.Regions[0], - CloudCredential: cred, - Type: "iaas", - IsController: false, - DefaultSeries: "warty", - Life: constants.ALIVE.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now(), - Valid: true, - }, - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, - }} - for _, m := range models { - c.Assert(s.Database.DB.Create(&m).Error, qt.IsNil) - } - - dbController := dbmodel.Controller{ - UUID: controller.UUID, - } - err = s.Database.GetController(context.Background(), &dbController) - c.Assert(err, qt.Equals, nil) - controller.Models = []dbmodel.Model{ - models[0], - } - c.Assert(dbController, qt.CmpEquals(cmpopts.IgnoreFields(dbmodel.Controller{}, "Models"), cmpopts.EquateEmpty()), controller) -} - func TestForEachControllerUnconfiguredDatabase(t *testing.T) { c := qt.New(t) @@ -289,7 +186,6 @@ func (s *dbSuite) TestUpdateController(c *qt.C) { UUID: "00000000-0000-0000-0000-0000-0000000000001", CloudName: "test-cloud", CloudRegion: "test-region", - Models: []dbmodel.Model{}, } err = s.Database.AddController(context.Background(), &controller) c.Assert(err, qt.Equals, nil) @@ -336,7 +232,6 @@ func (s *dbSuite) TestDeleteController(c *qt.C) { Name: "test-controller", UUID: "00000000-0000-0000-0000-0000-0000000000001", CloudName: "test-cloud", - Models: []dbmodel.Model{}, } err = s.Database.AddController(context.Background(), &controller) c.Assert(err, qt.Equals, nil) diff --git a/internal/db/export_test.go b/internal/db/export_test.go index c04ace351..7d8555cc3 100644 --- a/internal/db/export_test.go +++ b/internal/db/export_test.go @@ -3,11 +3,12 @@ package db var ( - JwksKind = jwksKind - JwksPublicKeyTag = jwksPublicKeyTag - JwksPrivateKeyTag = jwksPrivateKeyTag - JwksExpiryTag = jwksExpiryTag - OAuthKind = oauthKind - OAuthKeyTag = oauthKeyTag - NewUUID = &newUUID + JwksKind = jwksKind + JwksPublicKeyTag = jwksPublicKeyTag + JwksPrivateKeyTag = jwksPrivateKeyTag + JwksExpiryTag = jwksExpiryTag + OAuthKind = oauthKind + OAuthKeyTag = oauthKeyTag + OAuthSessionStoreSecretTag = oauthSessionStoreSecretTag + NewUUID = &newUUID ) diff --git a/internal/db/model.go b/internal/db/model.go index 68dcd5bab..3ed5a5ccc 100644 --- a/internal/db/model.go +++ b/internal/db/model.go @@ -181,3 +181,19 @@ func preloadModel(prefix string, db *gorm.DB) *gorm.DB { return db } + +// GetModelsByController retrieves a list of models hosted on the specified controller. +// Note that because we do not preload here, foreign key references will be empty. +func (d *Database) GetModelsByController(ctx context.Context, ctl dbmodel.Controller) ([]dbmodel.Model, error) { + const op = errors.Op("db.GetModelsByController") + + if err := d.ready(); err != nil { + return nil, errors.E(op, err) + } + var models []dbmodel.Model + db := d.DB.WithContext(ctx) + if err := db.Model(ctl).Association("Models").Find(&models); err != nil { + return nil, errors.E(op, dbError(err)) + } + return models, nil +} diff --git a/internal/db/model_test.go b/internal/db/model_test.go index 19a6db204..1f4848cca 100644 --- a/internal/db/model_test.go +++ b/internal/db/model_test.go @@ -7,11 +7,12 @@ import ( "database/sql" "sort" "testing" + "time" qt "github.com/frankban/quicktest" + "github.com/juju/juju/state" "gorm.io/gorm" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/db" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/errors" @@ -57,7 +58,6 @@ func (s *dbSuite) TestAddModel(c *qt.C) { UUID: "00000000-0000-0000-0000-0000-0000000000001", CloudName: "test-cloud", CloudRegion: "test-region", - Models: []dbmodel.Model{}, } err = s.Database.AddController(context.Background(), &controller) c.Assert(err, qt.Equals, nil) @@ -74,7 +74,7 @@ func (s *dbSuite) TestAddModel(c *qt.C) { CloudCredentialID: cred.ID, Type: "iaas", DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: db.Now(), @@ -127,7 +127,6 @@ func (s *dbSuite) TestGetModel(c *qt.C) { controller := dbmodel.Controller{ Name: "test-controller", UUID: "00000000-0000-0000-0000-0000-0000000000001", - Models: []dbmodel.Model{}, CloudName: "test-cloud", CloudRegion: "test-region", } @@ -150,7 +149,7 @@ func (s *dbSuite) TestGetModel(c *qt.C) { CloudCredential: cred, Type: "iaas", DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: db.Now(), @@ -231,7 +230,6 @@ func (s *dbSuite) TestUpdateModel(c *qt.C) { UUID: "00000000-0000-0000-0000-0000-0000000000001", CloudName: "test-cloud", CloudRegion: "test-region", - Models: []dbmodel.Model{}, } err = s.Database.AddController(context.Background(), &controller) c.Assert(err, qt.Equals, nil) @@ -244,7 +242,7 @@ func (s *dbSuite) TestUpdateModel(c *qt.C) { CloudCredentialID: cred.ID, Type: "iaas", DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: db.Now(), @@ -308,7 +306,6 @@ func (s *dbSuite) TestDeleteModel(c *qt.C) { UUID: "00000000-0000-0000-0000-0000-0000000000001", CloudName: "test-cloud", CloudRegion: "test-region", - Models: []dbmodel.Model{}, } err = s.Database.AddController(context.Background(), &controller) c.Assert(err, qt.Equals, nil) @@ -322,7 +319,7 @@ func (s *dbSuite) TestDeleteModel(c *qt.C) { CloudCredentialID: cred.ID, Type: "iaas", DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: db.Now(), @@ -405,7 +402,7 @@ func (s *dbSuite) TestGetModelsUsingCredential(c *qt.C) { CloudCredentialID: cred1.ID, Type: "iaas", DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: db.Now(), @@ -429,7 +426,7 @@ func (s *dbSuite) TestGetModelsUsingCredential(c *qt.C) { CloudCredentialID: cred2.ID, Type: "iaas", DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: db.Now(), @@ -459,7 +456,7 @@ func (s *dbSuite) TestGetModelsUsingCredential(c *qt.C) { CloudCredentialID: cred1.ID, Type: "iaas", DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: model1.Status, SLA: dbmodel.SLA{ Level: "unsupported", @@ -641,3 +638,102 @@ func (s *dbSuite) TestGetModelsByUUID(c *qt.C) { c.Check(models[2].UUID.String, qt.Equals, "00000002-0000-0000-0000-000000000003") c.Check(models[2].Controller.Name, qt.Not(qt.Equals), "") } + +func (s *dbSuite) TestGetModelsByController(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + + cloud := dbmodel.Cloud{ + Name: "test-cloud", + Type: "test-provider", + Regions: []dbmodel.CloudRegion{{ + Name: "test-region", + }}, + } + c.Assert(s.Database.DB.Create(&cloud).Error, qt.IsNil) + + controller := dbmodel.Controller{ + Name: "test-controller", + UUID: "00000000-0000-0000-0000-0000-0000000000001", + CloudName: "test-cloud", + CloudRegion: "test-region", + } + u, err := dbmodel.NewIdentity("bob@canonical.com") + c.Assert(err, qt.IsNil) + c.Assert(s.Database.DB.Create(&u).Error, qt.IsNil) + + cred := dbmodel.CloudCredential{ + Name: "test-cred", + Cloud: cloud, + Owner: *u, + AuthType: "empty", + } + c.Assert(s.Database.DB.Create(&cred).Error, qt.IsNil) + + err = s.Database.AddController(context.Background(), &controller) + c.Assert(err, qt.Equals, nil) + + models := []dbmodel.Model{{ + Name: "test-model-1", + UUID: sql.NullString{ + String: "00000001-0000-0000-0000-0000-000000000001", + Valid: true, + }, + Owner: *u, + Controller: controller, + CloudRegion: cloud.Regions[0], + CloudCredential: cred, + Type: "iaas", + IsController: true, + DefaultSeries: "focal", + Life: state.Alive.String(), + Status: dbmodel.Status{ + Status: "available", + Since: sql.NullTime{ + Time: time.Now(), + Valid: true, + }, + }, + SLA: dbmodel.SLA{ + Level: "unsupported", + }, + }, { + Name: "test-model-2", + UUID: sql.NullString{ + String: "00000001-0000-0000-0000-0000-000000000002", + Valid: true, + }, + Owner: *u, + Controller: controller, + CloudRegion: cloud.Regions[0], + CloudCredential: cred, + Type: "iaas", + IsController: false, + DefaultSeries: "focal", + Life: state.Alive.String(), + Status: dbmodel.Status{ + Status: "available", + Since: sql.NullTime{ + Time: time.Now(), + Valid: true, + }, + }, + SLA: dbmodel.SLA{ + Level: "unsupported", + }, + }} + for _, m := range models { + c.Assert(s.Database.DB.Create(&m).Error, qt.IsNil) + } + foundModels, err := s.Database.GetModelsByController(context.Background(), controller) + foundModelNames := []string{} + for _, m := range foundModels { + foundModelNames = append(foundModelNames, m.Name) + } + modelNames := []string{} + for _, m := range models { + modelNames = append(modelNames, m.Name) + } + c.Assert(err, qt.IsNil) + c.Assert(foundModelNames, qt.DeepEquals, modelNames) +} diff --git a/internal/db/secrets.go b/internal/db/secrets.go index 87434599f..e7074dc23 100644 --- a/internal/db/secrets.go +++ b/internal/db/secrets.go @@ -22,12 +22,13 @@ const ( passwordKey = "password" // These constants are used to create the appropriate identifiers for JWKS related data. - jwksKind = "jwks" - jwksPublicKeyTag = "jwksPublicKey" - jwksPrivateKeyTag = "jwksPrivateKey" - jwksExpiryTag = "jwksExpiry" - oauthKind = "oauth" - oauthKeyTag = "oauthKey" + jwksKind = "jwks" + jwksPublicKeyTag = "jwksPublicKey" + jwksPrivateKeyTag = "jwksPrivateKey" + jwksExpiryTag = "jwksExpiry" + oauthKind = "oauth" + oauthKeyTag = "oauthKey" + oauthSessionStoreSecretTag = "oauthSessionStoreSecret" ) // UpsertSecret stores secret information. @@ -287,10 +288,14 @@ func (d *Database) PutJWKSExpiry(ctx context.Context, expiry time.Time) error { func (d *Database) CleanupOAuthSecrets(ctx context.Context) error { const op = errors.Op("database.CleanupOAuthSecrets") secret := dbmodel.NewSecret(oauthKind, oauthKeyTag, nil) - err := d.DeleteSecret(ctx, &secret) - if err != nil { - zapctx.Error(ctx, "failed to cleanup OAUth key", zap.Error(err)) - return errors.E(op, err, "failed to cleanup OAUth key") + if err := d.DeleteSecret(ctx, &secret); err != nil { + zapctx.Error(ctx, "failed to cleanup OAuth key", zap.Error(err)) + return errors.E(op, err, "failed to cleanup OAuth key") + } + secret = dbmodel.NewSecret(oauthKind, oauthSessionStoreSecretTag, nil) + if err := d.DeleteSecret(ctx, &secret); err != nil { + zapctx.Error(ctx, "failed to cleanup OAuth session store secret", zap.Error(err)) + return errors.E(op, err, "failed to cleanup OAuth session store secret") } return nil } @@ -324,3 +329,33 @@ func (d *Database) PutOAuthSecret(ctx context.Context, raw []byte) error { secret := dbmodel.NewSecret(oauthKind, oauthKeyTag, oauthKey) return d.UpsertSecret(ctx, &secret) } + +// GetOAuthSessionStoreSecret returns the current secret used to store session tokens. +func (d *Database) GetOAuthSessionStoreSecret(ctx context.Context) ([]byte, error) { + const op = errors.Op("database.GetOAuthSessionStoreSecret") + secret := dbmodel.NewSecret(oauthKind, oauthSessionStoreSecretTag, nil) + err := d.GetSecret(ctx, &secret) + if err != nil { + zapctx.Error(ctx, "failed to get oauth session store secret", zap.Error(err)) + return nil, errors.E(op, err) + } + var pem []byte + err = json.Unmarshal(secret.Data, &pem) + if err != nil { + zapctx.Error(ctx, "failed to unmarshal pem data", zap.Error(err)) + return nil, errors.E(op, err) + } + return pem, nil +} + +// PutOAuthSessionStoreSecret puts a secret into the credentials store for secure storage of session tokens. +func (d *Database) PutOAuthSessionStoreSecret(ctx context.Context, raw []byte) error { + const op = errors.Op("database.PutOAuthSessionStoreSecret") + oauthSessionStoreSecret, err := json.Marshal(raw) + if err != nil { + zapctx.Error(ctx, "failed to marshal pem data", zap.Error(err)) + return errors.E(op, err, "failed to marshal oauth session store secret") + } + secret := dbmodel.NewSecret(oauthKind, oauthSessionStoreSecretTag, oauthSessionStoreSecret) + return d.UpsertSecret(ctx, &secret) +} diff --git a/internal/db/secrets_test.go b/internal/db/secrets_test.go index cc3f79af3..76bfaf991 100644 --- a/internal/db/secrets_test.go +++ b/internal/db/secrets_test.go @@ -307,3 +307,31 @@ func (s *dbSuite) TestGetOAuthSecretFailsIfNotFound(c *qt.C) { c.Assert(err, qt.ErrorMatches, "secret not found") c.Assert(retrieved, qt.IsNil) } + +func (s *dbSuite) TestPutAndGetOAuthSessionStoreSecret(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + key := []byte(uuid.NewString()) + c.Assert(s.Database.PutOAuthSessionStoreSecret(ctx, key), qt.IsNil) + + secret := dbmodel.Secret{} + tx := s.Database.DB.First(&secret) + c.Assert(tx.Error, qt.IsNil) + c.Assert(secret.Type, qt.Equals, db.OAuthKind) + c.Assert(secret.Tag, qt.Equals, db.OAuthSessionStoreSecretTag) + + retrievedKey, err := s.Database.GetOAuthSessionStoreSecret(ctx) + c.Assert(err, qt.IsNil) + c.Assert(retrievedKey, qt.DeepEquals, key) +} + +func (s *dbSuite) TestGetOAuthSessionStoreSecretFailsIfNotFound(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + + retrieved, err := s.Database.GetOAuthSessionStoreSecret(ctx) + c.Assert(err, qt.ErrorMatches, "secret not found") + c.Assert(retrieved, qt.IsNil) +} diff --git a/internal/dbmodel/cloud_test.go b/internal/dbmodel/cloud_test.go index 7d1351ec3..3f0d174bf 100644 --- a/internal/dbmodel/cloud_test.go +++ b/internal/dbmodel/cloud_test.go @@ -353,13 +353,17 @@ func TestCloudRegionControllers(t *testing.T) { c.Assert(err, qt.IsNil) c.Check(crcps, qt.HasLen, 2) c.Check(crcps[0].Controller, qt.DeepEquals, dbmodel.Controller{ - Model: ctl2.Model, + ID: ctl2.ID, + CreatedAt: ctl2.CreatedAt, + UpdatedAt: ctl2.UpdatedAt, Name: ctl2.Name, CloudName: ctl2.CloudName, CloudRegion: ctl2.CloudRegion, }) c.Check(crcps[1].Controller, qt.DeepEquals, dbmodel.Controller{ - Model: ctl1.Model, + ID: ctl1.ID, + CreatedAt: ctl1.CreatedAt, + UpdatedAt: ctl1.UpdatedAt, Name: ctl1.Name, CloudName: ctl1.CloudName, CloudRegion: ctl1.CloudRegion, @@ -370,13 +374,17 @@ func TestCloudRegionControllers(t *testing.T) { c.Assert(err, qt.IsNil) c.Check(crcps, qt.HasLen, 2) c.Check(crcps[0].Controller, qt.DeepEquals, dbmodel.Controller{ - Model: ctl1.Model, + ID: ctl1.ID, + CreatedAt: ctl1.CreatedAt, + UpdatedAt: ctl1.UpdatedAt, Name: ctl1.Name, CloudName: ctl1.CloudName, CloudRegion: ctl1.CloudRegion, }) c.Check(crcps[1].Controller, qt.DeepEquals, dbmodel.Controller{ - Model: ctl2.Model, + ID: ctl2.ID, + CreatedAt: ctl2.CreatedAt, + UpdatedAt: ctl2.UpdatedAt, Name: ctl2.Name, CloudName: ctl2.CloudName, CloudRegion: ctl2.CloudRegion, diff --git a/internal/dbmodel/controller.go b/internal/dbmodel/controller.go index 92297f36c..d88963ff4 100644 --- a/internal/dbmodel/controller.go +++ b/internal/dbmodel/controller.go @@ -4,8 +4,9 @@ package dbmodel import ( "database/sql" - "fmt" "net" + "strconv" + "time" jujuparams "github.com/juju/juju/rpc/params" "github.com/juju/names/v5" @@ -17,7 +18,10 @@ import ( // A controller represents a juju controller which is hosting models // within the JAAS system. type Controller struct { - gorm.Model + // Note that we do not use gorm.Model to avoid the use of soft-deletes. + ID uint `gorm:"primarykey"` + CreatedAt time.Time + UpdatedAt time.Time // Name is the name given to this controller. Name string `gorm:"not null;uniqueIndex"` @@ -46,6 +50,10 @@ type Controller struct { // name and port. PublicAddress string + // TLSHostname provides a hostname that will be used for TLS verfication. + // Useful for local dev to avoid TLS issues. + TLSHostname string `gorm:"column:tls_hostname"` + // CloudName is the name of the cloud which is hosting this // controller. CloudName string @@ -108,7 +116,7 @@ func (c Controller) ToAPIControllerInfo() apiparams.ControllerInfo { ci.PublicAddress = c.PublicAddress for _, hps := range c.Addresses { for _, hp := range hps { - ci.APIAddresses = append(ci.APIAddresses, fmt.Sprintf("%s:%d", hp.Value, hp.Port)) + ci.APIAddresses = append(ci.APIAddresses, net.JoinHostPort(hp.Value, strconv.Itoa(hp.Port))) } } ci.CACertificate = c.CACertificate diff --git a/internal/dbmodel/model.go b/internal/dbmodel/model.go index e589fae15..f3838a79e 100644 --- a/internal/dbmodel/model.go +++ b/internal/dbmodel/model.go @@ -37,8 +37,9 @@ type Model struct { ControllerID uint Controller Controller + // (Unused) Currently model migrations are completed manually. // MigrationControllerID is the controller that a model is migrating to. - // This is only filled if the new controller is outside JIMM. + // This is only filled if the new controller is within JIMM. MigrationControllerID sql.NullInt32 // CloudRegion is the cloud-region hosting the model. diff --git a/internal/dbmodel/model_test.go b/internal/dbmodel/model_test.go index 411f7ce0e..923a010a5 100644 --- a/internal/dbmodel/model_test.go +++ b/internal/dbmodel/model_test.go @@ -10,10 +10,10 @@ import ( qt "github.com/frankban/quicktest" "github.com/juju/juju/core/life" jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/juju/state" "github.com/juju/names/v5" "gorm.io/gorm" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/dbmodel" ) @@ -82,7 +82,7 @@ func TestModel(t *testing.T) { Type: "iaas", IsController: false, DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: sql.NullTime{ @@ -148,7 +148,7 @@ func TestModelUniqueConstraint(t *testing.T) { Type: "iaas", IsController: false, DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: sql.NullTime{ @@ -175,7 +175,7 @@ func TestModelUniqueConstraint(t *testing.T) { Type: "iaas", IsController: false, DefaultSeries: "jammy", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: sql.NullTime{ @@ -222,7 +222,7 @@ func TestToJujuModel(t *testing.T) { Type: "iaas", IsController: false, DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: sql.NullTime{ @@ -263,7 +263,7 @@ func TestToJujuModelSummary(t *testing.T) { Type: "iaas", IsController: false, DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: sql.NullTime{ @@ -293,7 +293,7 @@ func TestToJujuModelSummary(t *testing.T) { CloudRegion: "test-region", CloudCredentialTag: "cloudcred-test-cloud_bob@canonical.com_test-cred", OwnerTag: "user-bob@canonical.com", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: jujuparams.EntityStatus{ Status: "available", Since: &now, @@ -369,7 +369,7 @@ func TestModelFromJujuModelInfo(t *testing.T) { CloudCredentialTag: "cloudcred-test-cloud_bob@canonical.com_test-cred", CloudCredentialValidity: nil, OwnerTag: "user-bob@canonical.com", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: jujuparams.EntityStatus{ Status: "available", Since: &now, @@ -428,7 +428,7 @@ func TestModelFromJujuModelInfo(t *testing.T) { Type: "iaas", IsController: false, DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: sql.NullTime{ @@ -448,7 +448,7 @@ func TestModelFromJujuModelUpdate(t *testing.T) { info := jujuparams.ModelUpdate{ Name: "test-model", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: jujuparams.StatusInfo{ Current: "available", Since: &now, @@ -462,7 +462,7 @@ func TestModelFromJujuModelUpdate(t *testing.T) { model.FromJujuModelUpdate(info) c.Assert(model, qt.DeepEquals, dbmodel.Model{ Name: "test-model", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: sql.NullTime{ diff --git a/internal/dbmodel/sql/postgres/1_8.sql b/internal/dbmodel/sql/postgres/1_8.sql new file mode 100644 index 000000000..d27afcb3d --- /dev/null +++ b/internal/dbmodel/sql/postgres/1_8.sql @@ -0,0 +1,4 @@ +-- 1_8.sql is a migration that adds a tls_hostname column to the controller table. +ALTER TABLE controllers ADD COLUMN tls_hostname TEXT; + +UPDATE versions SET major=1, minor=8 WHERE component='jimmdb'; diff --git a/internal/dbmodel/sql/postgres/1_9.sql b/internal/dbmodel/sql/postgres/1_9.sql new file mode 100644 index 000000000..d43ad6591 --- /dev/null +++ b/internal/dbmodel/sql/postgres/1_9.sql @@ -0,0 +1,6 @@ +-- 1_9.sql is a migration that deletes soft-deleted controllers and +-- drops the deleted_at column from the controllers table. +DELETE FROM controllers WHERE deleted_at IS NOT null; +ALTER TABLE controllers DROP COLUMN deleted_at; + +UPDATE versions SET major=1, minor=9 WHERE component='jimmdb'; diff --git a/internal/dbmodel/version.go b/internal/dbmodel/version.go index 2c113e675..1649904f4 100644 --- a/internal/dbmodel/version.go +++ b/internal/dbmodel/version.go @@ -20,7 +20,7 @@ const ( // Minor is the minor version of the model described in the dbmodel // package. It should be incremented for any change made to the // database model from database model in a released JIMM. - Minor = 7 + Minor = 9 ) type Version struct { diff --git a/internal/jimm/access.go b/internal/jimm/access.go index b02c2b2a2..18465af54 100644 --- a/internal/jimm/access.go +++ b/internal/jimm/access.go @@ -419,7 +419,7 @@ func (j *JIMM) ToJAASTag(ctx context.Context, tag *ofganames.Tag) (string, error } err := j.Database.GetModel(ctx, &model) if err != nil { - return "", errors.E(err, "failed to fetch model information") + return "", errors.E(err, fmt.Sprintf("failed to fetch model information: %s", model.UUID.String)) } modelString := names.ModelTagKind + "-" + model.Controller.Name + ":" + model.OwnerIdentityName + "/" + model.Name if tag.Relation.String() != "" { @@ -432,7 +432,7 @@ func (j *JIMM) ToJAASTag(ctx context.Context, tag *ofganames.Tag) (string, error } err := j.Database.GetApplicationOffer(ctx, &ao) if err != nil { - return "", errors.E(err, "failed to fetch application offer information") + return "", errors.E(err, fmt.Sprintf("failed to fetch application offer information: %s", ao.UUID)) } aoString := names.ApplicationOfferTagKind + "-" + ao.Model.Controller.Name + ":" + ao.Model.OwnerIdentityName + "/" + ao.Model.Name + "." + ao.Name if tag.Relation.String() != "" { @@ -445,7 +445,7 @@ func (j *JIMM) ToJAASTag(ctx context.Context, tag *ofganames.Tag) (string, error } err := j.Database.GetGroup(ctx, &group) if err != nil { - return "", errors.E(err, "failed to fetch group information") + return "", errors.E(err, fmt.Sprintf("failed to fetch group information: %s", group.UUID)) } groupString := jimmnames.GroupTagKind + "-" + group.Name if tag.Relation.String() != "" { @@ -458,7 +458,7 @@ func (j *JIMM) ToJAASTag(ctx context.Context, tag *ofganames.Tag) (string, error } err := j.Database.GetCloud(ctx, &cloud) if err != nil { - return "", errors.E(err, "failed to fetch group information") + return "", errors.E(err, fmt.Sprintf("failed to fetch cloud information: %s", cloud.Name)) } cloudString := names.CloudTagKind + "-" + cloud.Name if tag.Relation.String() != "" { diff --git a/internal/jimm/access_test.go b/internal/jimm/access_test.go index cda526635..e27fb7268 100644 --- a/internal/jimm/access_test.go +++ b/internal/jimm/access_test.go @@ -14,9 +14,9 @@ import ( "github.com/google/uuid" "github.com/juju/juju/core/crossmodel" jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/juju/state" "github.com/juju/names/v5" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/db" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/errors" @@ -807,7 +807,7 @@ func createTestControllerEnvironment(ctx context.Context, c *qt.C, db db.Databas ControllerID: controller.ID, CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred.ID, - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: sql.NullTime{ diff --git a/internal/jimm/cloudcredential_test.go b/internal/jimm/cloudcredential_test.go index 0541c3b1b..3ca26a310 100644 --- a/internal/jimm/cloudcredential_test.go +++ b/internal/jimm/cloudcredential_test.go @@ -1781,3 +1781,11 @@ func (s testCloudCredentialAttributeStore) GetOAuthSecret(ctx context.Context) ( func (s testCloudCredentialAttributeStore) PutOAuthSecret(ctx context.Context, raw []byte) error { return errors.E(errors.CodeNotImplemented) } + +func (s testCloudCredentialAttributeStore) GetOAuthSessionStoreSecret(ctx context.Context) ([]byte, error) { + return nil, errors.E(errors.CodeNotImplemented) +} + +func (s testCloudCredentialAttributeStore) PutOAuthSessionStoreSecret(ctx context.Context, raw []byte) error { + return errors.E(errors.CodeNotImplemented) +} diff --git a/internal/jimm/controller.go b/internal/jimm/controller.go index c1b42141d..a995b2f01 100644 --- a/internal/jimm/controller.go +++ b/internal/jimm/controller.go @@ -18,7 +18,6 @@ import ( "go.uber.org/zap" "gopkg.in/macaroon.v2" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/db" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/errors" @@ -608,7 +607,7 @@ func (j *JIMM) UpdateMigratedModel(ctx context.Context, user *openfga.User, mode // InitiateMigration triggers the migration of the specified model to a target controller. // externalMigration indicates whether this model is moving to a controller managed by // JIMM or not. -func (j *JIMM) InitiateMigration(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec, newControllerID uint) (jujuparams.InitiateMigrationResult, error) { +func (j *JIMM) InitiateMigration(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error) { const op = errors.Op("jimm.InitiateMigration") result := jujuparams.InitiateMigrationResult{ @@ -672,18 +671,5 @@ func (j *JIMM) InitiateMigration(ctx context.Context, user *openfga.User, spec j if err != nil { return result, errors.E(op, err) } - // Track the model migration info - if newControllerID == 0 { - model.Life = constants.MIGRATING_AWAY.String() - } else { - model.Life = constants.MIGRATING_INTERNAL.String() - model.MigrationControllerID = sql.NullInt32{Int32: int32(newControllerID), Valid: true} - } - err = j.Database.UpdateModel(ctx, &model) - if err != nil { - zapctx.Error(ctx, "failed to update model with migration info", zap.Error(err)) - return result, errors.E(op, "migration started but failed to queue JIMM automatic update", err) - } - return result, nil } diff --git a/internal/jimm/controller_test.go b/internal/jimm/controller_test.go index ec63015ba..a2d775e6e 100644 --- a/internal/jimm/controller_test.go +++ b/internal/jimm/controller_test.go @@ -18,11 +18,11 @@ import ( "github.com/juju/juju/core/life" "github.com/juju/juju/core/status" jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/juju/state" "github.com/juju/names/v5" semversion "github.com/juju/version" "gopkg.in/macaroon.v2" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/db" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/errors" @@ -119,7 +119,7 @@ func TestAddController(t *testing.T) { ms.CloudTag = "cloud-aws" ms.CloudRegion = "eu-west-1" ms.OwnerTag = "user-admin" - ms.Life = life.Value(constants.ALIVE.String()) + ms.Life = life.Value(state.Alive.String()) ms.Status = jujuparams.EntityStatus{ Status: "available", } @@ -285,7 +285,7 @@ func TestAddControllerWithVault(t *testing.T) { ms.CloudTag = "cloud-aws" ms.CloudRegion = "eu-west-1" ms.OwnerTag = "user-admin" - ms.Life = life.Value(constants.ALIVE.String()) + ms.Life = life.Value(state.Alive.String()) ms.Status = jujuparams.EntityStatus{ Status: "available", } @@ -543,7 +543,7 @@ func TestImportModel(t *testing.T) { ModelUUID: "00000002-0000-0000-0000-000000000001", Name: "test-model", Owner: "alice@canonical.com", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), ControllerUUID: "00000001-0000-0000-0000-000000000001", Status: jujuparams.StatusInfo{ Current: "available", @@ -562,7 +562,7 @@ func TestImportModel(t *testing.T) { Name: "app-1", Exposed: true, CharmURL: "cs:app-1", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), MinUnits: 1, WorkloadVersion: "2", }, @@ -570,7 +570,7 @@ func TestImportModel(t *testing.T) { Entity: &jujuparams.MachineInfo{ ModelUUID: "00000002-0000-0000-0000-000000000001", Id: "machine-1", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Hostname: "test-machine-1", }, }, { @@ -624,7 +624,7 @@ func TestImportModel(t *testing.T) { }, Type: "test-type", DefaultSeries: "test-series", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Info: "updated status message", @@ -712,7 +712,7 @@ func TestImportModel(t *testing.T) { }, Type: "test-type", DefaultSeries: "test-series", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Info: "test-info", @@ -1624,7 +1624,7 @@ func TestInitiateMigration(t *testing.T) { user := test.user(client) - result, err := j.InitiateMigration(context.Background(), user, test.spec, 0) + result, err := j.InitiateMigration(context.Background(), user, test.spec) if test.expectedError == "" { c.Assert(err, qt.IsNil) c.Assert(result, qt.DeepEquals, test.expectedResult) diff --git a/internal/jimm/credentials/credentials.go b/internal/jimm/credentials/credentials.go index 4f28d70ee..bfeacd987 100644 --- a/internal/jimm/credentials/credentials.go +++ b/internal/jimm/credentials/credentials.go @@ -63,4 +63,10 @@ type CredentialStore interface { // PutOAuthSecret puts a HS256 (symmetric encryption) secret into the credentials store for signing OAuth session tokens. PutOAuthSecret(ctx context.Context, raw []byte) error + + // GetOAuthSessionStoreSecret returns the current secret used to store session tokens. + GetOAuthSessionStoreSecret(ctx context.Context) ([]byte, error) + + // PutOAuthSessionStoreSecret puts a secret into the credentials store for secure storage of session tokens. + PutOAuthSessionStoreSecret(ctx context.Context, raw []byte) error } diff --git a/internal/jimm/jimm.go b/internal/jimm/jimm.go index 8dc0c1484..211978dce 100644 --- a/internal/jimm/jimm.go +++ b/internal/jimm/jimm.go @@ -34,8 +34,8 @@ import ( ) var ( - initiateMigration = func(ctx context.Context, j *JIMM, user *openfga.User, spec jujuparams.MigrationSpec, targetControllerID uint) (jujuparams.InitiateMigrationResult, error) { - return j.InitiateMigration(ctx, user, spec, targetControllerID) + initiateMigration = func(ctx context.Context, j *JIMM, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error) { + return j.InitiateMigration(ctx, user, spec) } ) @@ -527,8 +527,12 @@ func (j *JIMM) RemoveController(ctx context.Context, user *openfga.User, control return errors.E(errors.CodeStillAlive, "controller is still alive") } + models, err := db.GetModelsByController(ctx, c) + if err != nil { + return err + } // Delete its models first. - for _, model := range c.Models { + for _, model := range models { err := db.DeleteModel(ctx, &model) if err != nil { return err @@ -618,7 +622,7 @@ func fillMigrationTarget(db db.Database, credStore credentials.CredentialStore, func (j *JIMM) InitiateInternalMigration(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetController string) (jujuparams.InitiateMigrationResult, error) { const op = errors.Op("jimm.InitiateInternalMigration") - migrationTarget, controllerID, err := fillMigrationTarget(j.Database, j.CredentialStore, targetController) + migrationTarget, _, err := fillMigrationTarget(j.Database, j.CredentialStore, targetController) if err != nil { return jujuparams.InitiateMigrationResult{}, errors.E(op, err) } @@ -634,7 +638,7 @@ func (j *JIMM) InitiateInternalMigration(ctx context.Context, user *openfga.User return jujuparams.InitiateMigrationResult{}, errors.E(op, err) } spec := jujuparams.MigrationSpec{ModelTag: modelTag.String(), TargetInfo: migrationTarget} - result, err := initiateMigration(ctx, j, user, spec, controllerID) + result, err := initiateMigration(ctx, j, user, spec) if err != nil { return result, errors.E(op, err) } diff --git a/internal/jimm/jimm_test.go b/internal/jimm/jimm_test.go index 8d2062e56..3d6223f7b 100644 --- a/internal/jimm/jimm_test.go +++ b/internal/jimm/jimm_test.go @@ -468,7 +468,6 @@ func TestRemoveController(t *testing.T) { env := jimmtest.ParseEnvironment(c, removeControllerTestEnv) env.PopulateDB(c, j.Database) - // env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client) dbUser := env.User(test.user).DBObject(c, j.Database) user := openfga.NewUser(&dbUser, nil) @@ -500,6 +499,92 @@ func TestRemoveController(t *testing.T) { } } +const removeAndAddControllerTestEnv = `clouds: +- name: test-cloud + type: test-provider + regions: + - name: test-cloud-region +cloud-credentials: +- owner: alice@canonical.com + name: cred-1 + cloud: test-cloud +users: +- username: alice@canonical.com + controller-access: superuser +- username: bob@canonical.com + controller-access: login +- username: eve@canonical.com + controller-access: "no-access" +controllers: +- name: controller-1 + uuid: 00000001-0000-0000-0000-000000000001 + cloud: test-cloud + region: test-cloud-region +models: +- name: model-1 + type: iaas + uuid: 00000002-0000-0000-0000-000000000001 + controller: controller-1 + default-series: warty + cloud: test-cloud + region: test-cloud-region + cloud-credential: cred-1 + owner: alice@canonical.com + life: alive + status: + status: available + info: "OK!" + since: 2020-02-20T20:02:20Z + users: + - user: alice@canonical.com + access: admin + - user: bob@canonical.com + access: write + - user: charlie@canonical.com + access: read + sla: + level: unsupported + agent-version: 1.2.3 +` + +func TestRemoveAndAddController(t *testing.T) { + c := qt.New(t) + ctx := context.Background() + now := time.Now().UTC().Round(time.Millisecond) + + j := &jimm.JIMM{ + UUID: uuid.NewString(), + Database: db.Database{ + DB: jimmtest.PostgresDB(c, func() time.Time { return now }), + }, + } + + err := j.Database.Migrate(ctx, false) + c.Assert(err, qt.IsNil) + + env := jimmtest.ParseEnvironment(c, removeAndAddControllerTestEnv) + env.PopulateDB(c, j.Database) + controller := env.Controllers[0] + + dbUser := env.User("alice@canonical.com").DBObject(c, j.Database) + user := openfga.NewUser(&dbUser, nil) + user.JimmAdmin = true + + err = j.RemoveController(ctx, user, "controller-1", true) + c.Assert(err, qt.Equals, nil) + ctls, err := j.ListControllers(ctx, user) + c.Assert(err, qt.Equals, nil) + c.Assert(len(ctls), qt.Equals, 0) + // Recreate the controller. + ctlDbObject := controller.DBObject(c, j.Database) + ctlDbObject.ID = 0 + err = j.Database.AddController(ctx, &ctlDbObject) + c.Assert(err, qt.Equals, nil) + ctls, err = j.ListControllers(ctx, user) + c.Assert(err, qt.Equals, nil) + c.Assert(len(ctls), qt.Equals, 1) +} + const fullModelStatusTestEnv = `clouds: - name: test-cloud type: test-provider @@ -780,7 +865,7 @@ func TestInitiateInternalMigration(t *testing.T) { for _, test := range tests { c.Run(test.about, func(c *qt.C) { - c.Patch(jimm.InitiateMigration, func(ctx context.Context, j *jimm.JIMM, user *openfga.User, spec jujuparams.MigrationSpec, targetID uint) (jujuparams.InitiateMigrationResult, error) { + c.Patch(jimm.InitiateMigration, func(ctx context.Context, j *jimm.JIMM, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error) { return jujuparams.InitiateMigrationResult{}, nil }) store := jimmtest.NewInMemoryCredentialStore() diff --git a/internal/jimm/model.go b/internal/jimm/model.go index f83c70265..1c275d9f8 100644 --- a/internal/jimm/model.go +++ b/internal/jimm/model.go @@ -12,12 +12,12 @@ import ( jujupermission "github.com/juju/juju/core/permission" jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/juju/state" "github.com/juju/names/v5" "github.com/juju/zaputil" "github.com/juju/zaputil/zapctx" "go.uber.org/zap" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/errors" "github.com/canonical/jimm/internal/openfga" @@ -1008,7 +1008,7 @@ func (j *JIMM) DestroyModel(ctx context.Context, user *openfga.User, mt names.Mo if err := api.DestroyModel(ctx, mt, destroyStorage, force, maxWait, timeout); err != nil { return err } - m.Life = constants.DYING.String() + m.Life = state.Dying.String() if err := j.Database.UpdateModel(ctx, m); err != nil { // If the database fails to update don't worry too much the // monitor should catch it. diff --git a/internal/jimm/model_status_parser_test.go b/internal/jimm/model_status_parser_test.go index 9933cf05a..1da89bf9f 100644 --- a/internal/jimm/model_status_parser_test.go +++ b/internal/jimm/model_status_parser_test.go @@ -10,8 +10,8 @@ import ( "github.com/juju/juju/core/life" "github.com/juju/juju/core/status" jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/juju/state" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/db" "github.com/canonical/jimm/internal/errors" "github.com/canonical/jimm/internal/jimm" @@ -363,7 +363,7 @@ func TestQueryModelsJq(t *testing.T) { MountPoint: "/home/ubuntu/myapp/.data", ReadOnly: false, }, - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), }, }, }, diff --git a/internal/jimm/model_test.go b/internal/jimm/model_test.go index 15cb110ff..9ac4106fd 100644 --- a/internal/jimm/model_test.go +++ b/internal/jimm/model_test.go @@ -16,11 +16,11 @@ import ( "github.com/google/uuid" "github.com/juju/juju/core/life" jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/juju/state" "github.com/juju/names/v5" "github.com/juju/version/v2" "sigs.k8s.io/yaml" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/db" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/errors" @@ -257,7 +257,7 @@ users: Name: "test-credential-1", AuthType: "empty", }, - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "started", Info: "running a test", @@ -371,7 +371,7 @@ users: Name: "test-credential-1", AuthType: "empty", }, - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "started", Info: "running a test", @@ -464,7 +464,7 @@ users: Name: "test-credential-1", AuthType: "empty", }, - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "started", Info: "running a test", @@ -564,7 +564,7 @@ users: Name: "test-credential-1", AuthType: "empty", }, - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "started", Info: "running a test", @@ -1066,7 +1066,7 @@ var modelInfoTests = []struct { CloudRegion: "test-cloud-region", CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), OwnerTag: names.NewUserTag("alice@canonical.com").String(), - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: jujuparams.EntityStatus{ Status: "available", Info: "OK!", @@ -1120,7 +1120,7 @@ var modelInfoTests = []struct { CloudRegion: "test-cloud-region", CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), OwnerTag: names.NewUserTag("alice@canonical.com").String(), - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: jujuparams.EntityStatus{ Status: "available", Info: "OK!", @@ -1168,7 +1168,7 @@ var modelInfoTests = []struct { CloudRegion: "test-cloud-region", CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), OwnerTag: names.NewUserTag("alice@canonical.com").String(), - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: jujuparams.EntityStatus{ Status: "available", Info: "OK!", @@ -1211,7 +1211,7 @@ var modelInfoTests = []struct { CloudRegion: "test-cloud-region", CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), OwnerTag: names.NewUserTag("alice@canonical.com").String(), - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: jujuparams.EntityStatus{ Status: "available", Info: "OK!", @@ -1257,7 +1257,7 @@ func TestModelInfo(t *testing.T) { mi.CloudRegion = "test-cloud-region" mi.CloudCredentialTag = names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String() mi.OwnerTag = names.NewUserTag("alice@canonical.com").String() - mi.Life = life.Value(constants.ALIVE.String()) + mi.Life = life.Value(state.Alive.String()) mi.Status = jujuparams.EntityStatus{ Status: "available", Info: "OK!", @@ -1378,7 +1378,7 @@ var modelStatusTests = []struct { if ms.ModelTag != names.NewModelTag("00000002-0000-0000-0000-000000000001").String() { return errors.E("incorrect model tag") } - ms.Life = life.Value(constants.ALIVE.String()) + ms.Life = life.Value(state.Alive.String()) ms.Type = "iaas" ms.HostedMachineCount = 10 ms.ApplicationCount = 3 @@ -1390,7 +1390,7 @@ var modelStatusTests = []struct { uuid: "00000002-0000-0000-0000-000000000001", expectModelStatus: &jujuparams.ModelStatus{ ModelTag: names.NewModelTag("00000002-0000-0000-0000-000000000001").String(), - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Type: "iaas", HostedMachineCount: 10, ApplicationCount: 3, @@ -1614,7 +1614,7 @@ func TestForEachUserModel(t *testing.T) { CloudRegion: "test-cloud-region", CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), OwnerTag: names.NewUserTag("alice@canonical.com").String(), - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: jujuparams.EntityStatus{ Status: "available", Info: "OK!", @@ -1648,7 +1648,7 @@ func TestForEachUserModel(t *testing.T) { CloudRegion: "test-cloud-region", CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), OwnerTag: names.NewUserTag("alice@canonical.com").String(), - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: jujuparams.EntityStatus{ Status: "available", Info: "OK!", @@ -1682,7 +1682,7 @@ func TestForEachUserModel(t *testing.T) { CloudRegion: "test-cloud-region", CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), OwnerTag: names.NewUserTag("alice@canonical.com").String(), - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: jujuparams.EntityStatus{ Status: "available", Info: "OK!", @@ -2929,7 +2929,7 @@ func TestDestroyModel(t *testing.T) { } err = j.Database.GetModel(ctx, &m) c.Assert(err, qt.IsNil) - c.Check(m.Life, qt.Equals, constants.DYING.String()) + c.Check(m.Life, qt.Equals, state.Dying.String()) }) } } diff --git a/internal/jimm/watcher.go b/internal/jimm/watcher.go index 2ec0e4bcc..3ba56ae2b 100644 --- a/internal/jimm/watcher.go +++ b/internal/jimm/watcher.go @@ -7,13 +7,12 @@ import ( "database/sql" "time" - "github.com/juju/juju/core/migration" jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/juju/state" "github.com/juju/names/v5" "github.com/juju/zaputil/zapctx" "go.uber.org/zap" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/db" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/errors" @@ -228,67 +227,8 @@ func (w *Watcher) watchController(ctx context.Context, ctl *dbmodel.Controller) } defer api.AllModelWatcherStop(ctx, id) - checkMigratingModel := func(m *dbmodel.Model) error { - // models that were in the migrating state may no longer be on - // the controller, here we will update them once migration has completed. - modelMigrating := m.Life == constants.MIGRATING_AWAY.String() || m.Life == constants.MIGRATING_INTERNAL.String() - if !modelMigrating { - return nil - } - mi := jujuparams.ModelInfo{ - UUID: m.UUID.String, - } - if err := api.ModelInfo(ctx, &mi); err == nil { - if mi.Migration == nil { - return nil - } - migrationPhase, ok := migration.ParsePhase(mi.Migration.Status) - if !ok { - zapctx.Error(ctx, "invalid phase received", zap.String("phase", mi.Migration.Status)) - return nil - } - zapctx.Info(ctx, "model migration in progress", zap.String("model", m.Name), zap.String("phase", migrationPhase.String())) - // Without a clean way to check for migration failure, we opt to check for these two states - // which are currently the final states for any failed migrations. - if migrationPhase == migration.ABORTDONE || migrationPhase == migration.REAPFAILED { - // Clean up migration info - m.MigrationControllerID = sql.NullInt32{Int32: 0, Valid: false} - m.Life = constants.ALIVE.String() - if err := w.Database.UpdateModel(ctx, m); err != nil { - zapctx.Error(ctx, "failed to update migrating model info", zap.Error(err)) - return errors.E(op, err) - } - return nil - } - } else { - // If we get an error then we have reached migration completion. - misingOrRedirectError := errors.ErrorCode(err) == errors.CodeNotFound || errors.ErrorCode(err) == errors.CodeRedirect - if !misingOrRedirectError { - return nil - } - // Model undergoing internal migration needs an update to its parent controller. - if m.Life == constants.MIGRATING_INTERNAL.String() { - m.ControllerID = uint(m.MigrationControllerID.Int32) - m.MigrationControllerID = sql.NullInt32{Int32: 0, Valid: false} - m.Life = constants.ALIVE.String() - if err := w.Database.UpdateModel(ctx, m); err != nil { - zapctx.Error(ctx, "failed to update migrating model info", zap.Error(err)) - return errors.E(op, err) - } - return nil - } else { - // Model migrating to controller not managed by JIMM should now be deleted. - if err := w.Database.DeleteModel(ctx, m); err != nil { - return errors.E(op, err) - } - return nil - } - } - return nil - } - checkDyingModel := func(m *dbmodel.Model) error { - if m.Life == constants.DYING.String() || m.Life == constants.DEAD.String() { + if m.Life == state.Dying.String() || m.Life == state.Dead.String() { // models that were in the dying state may no // longer be on the controller, check if it should // be immediately deleted. @@ -314,7 +254,7 @@ func (w *Watcher) watchController(ctx context.Context, ctl *dbmodel.Controller) // modelStates contains the set of models running on the // controller that JIMM is interested in. The function also // check for any dying models and deletes them where necessary. - modelStates, err := w.checkControllerModels(ctx, ctl, checkDyingModel, checkMigratingModel) + modelStates, err := w.checkControllerModels(ctx, ctl, checkDyingModel) if err != nil { return errors.E(op, err) } @@ -369,6 +309,7 @@ func (w *Watcher) watchController(ctx context.Context, ctl *dbmodel.Controller) continue } if v.changed { + v.changed = false // Update changed model. err := w.Database.Transaction(func(tx *db.Database) error { m := dbmodel.Model{ @@ -558,7 +499,7 @@ func (w *Watcher) deleteModel(ctx context.Context, model *dbmodel.Model) error { return err } } - if !(model.Life == constants.DYING.String() || model.Life == constants.DEAD.String()) { + if !(model.Life == state.Dying.String() || model.Life == state.Dead.String()) { // If the model hasn't been marked as dying, don't remove it. return nil } diff --git a/internal/jimm/watcher_test.go b/internal/jimm/watcher_test.go index 05056d80c..2fcbfd962 100644 --- a/internal/jimm/watcher_test.go +++ b/internal/jimm/watcher_test.go @@ -13,11 +13,10 @@ import ( qt "github.com/frankban/quicktest" "github.com/juju/juju/core/instance" "github.com/juju/juju/core/life" - "github.com/juju/juju/core/migration" jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/juju/state" "github.com/juju/names/v5" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/db" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/errors" @@ -218,7 +217,7 @@ var watcherTests = []struct { Name: "app-1", Exposed: true, CharmURL: "cs:app-1", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), MinUnits: 1, WorkloadVersion: "2", }, @@ -358,7 +357,7 @@ var watcherTests = []struct { ModelUUID: "00000002-0000-0000-0000-000000000001", Name: "model-1", Owner: "alice@canonical.com", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), ControllerUUID: "00000001-0000-0000-0000-000000000001", Status: jujuparams.StatusInfo{ Current: "available", @@ -398,7 +397,7 @@ var watcherTests = []struct { Name: "model-1", Type: "iaas", DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Info: "updated status message", @@ -494,7 +493,7 @@ var watcherTests = []struct { Name: "model-1", Type: "iaas", DefaultSeries: "warty", - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Info: "OK!", @@ -870,209 +869,6 @@ func TestWatcherRemoveDyingModelsOnStartup(t *testing.T) { c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeNotFound) } -const testMigratingModelsWatcherEnv = `clouds: -- name: test-cloud - type: test-provider - regions: - - name: test-cloud-region -cloud-credentials: -- owner: alice@canonical.com - name: cred-1 - cloud: test-cloud -controllers: -- name: controller-1 - uuid: 00000001-0000-0000-0000-000000000001 - cloud: test-cloud - region: test-cloud-region -- name: controller-2 - uuid: 00000001-0000-0000-0000-000000000002 - cloud: test-cloud - region: test-cloud-region -models: -- name: model-1 - type: iaas - uuid: 00000002-0000-0000-0000-000000000001 - controller: controller-1 - migration-controller: controller-2 - default-series: warty - cloud: test-cloud - region: test-cloud-region - cloud-credential: cred-1 - owner: alice@canonical.com - life: migrating-internal -- name: model-2 - type: iaas - uuid: 00000002-0000-0000-0000-000000000002 - controller: controller-1 - default-series: warty - cloud: test-cloud - region: test-cloud-region - cloud-credential: cred-1 - owner: alice@canonical.com - life: migrating-away -` - -func TestWatcherRemoveMigratingModels(t *testing.T) { - c := qt.New(t) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - w := &jimm.Watcher{ - Pubsub: &testPublisher{}, - Database: db.Database{ - DB: jimmtest.PostgresDB(c, nil), - }, - Dialer: &jimmtest.Dialer{ - API: &jimmtest.API{ - AllModelWatcherNext_: func(_ context.Context, _ string) ([]jujuparams.Delta, error) { - cancel() - <-ctx.Done() - return nil, ctx.Err() - }, - ModelInfo_: func(_ context.Context, info *jujuparams.ModelInfo) error { - switch info.UUID { - default: - c.Errorf("unexpected model uuid: %s", info.UUID) - case "00000002-0000-0000-0000-000000000001": - case "00000002-0000-0000-0000-000000000002": - } - return errors.E(errors.CodeNotFound) - }, - WatchAllModels_: func(ctx context.Context) (string, error) { - return "1234", nil - }, - }, - }, - } - env := jimmtest.ParseEnvironment(c, testMigratingModelsWatcherEnv) - err := w.Database.Migrate(ctx, false) - c.Assert(err, qt.IsNil) - env.PopulateDB(c, w.Database) - ctl := env.Controllers[0].DBObject(c, w.Database) - - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - err := jimm.WatchController(w, ctx, &ctl) - checkIfContextCanceled(c, ctx, err) - }() - wg.Wait() - - modelInternalMigrated := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - } - err = w.Database.GetModel(context.Background(), &modelInternalMigrated) - c.Assert(err, qt.IsNil) - c.Assert(modelInternalMigrated.Controller.Name, qt.Equals, "controller-2") - - modelExternalMigrated := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000002", - Valid: true, - }, - } - err = w.Database.GetModel(context.Background(), &modelExternalMigrated) - c.Assert(errors.ErrorCode(err), qt.Equals, errors.CodeNotFound) -} - -const testFailedMigratingModelsWatcherEnv = `clouds: -- name: test-cloud - type: test-provider - regions: - - name: test-cloud-region -cloud-credentials: -- owner: alice@canonical.com - name: cred-1 - cloud: test-cloud -controllers: -- name: controller-1 - uuid: 00000001-0000-0000-0000-000000000001 - cloud: test-cloud - region: test-cloud-region -- name: controller-2 - uuid: 00000001-0000-0000-0000-000000000002 - cloud: test-cloud - region: test-cloud-region -models: -- name: model-1 - type: iaas - uuid: 00000002-0000-0000-0000-000000000001 - controller: controller-1 - migration-controller: controller-2 - default-series: warty - cloud: test-cloud - region: test-cloud-region - cloud-credential: cred-1 - owner: alice@canonical.com - life: migrating-internal -` - -func TestWatcherCleansFailedMigrations(t *testing.T) { - c := qt.New(t) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - w := &jimm.Watcher{ - Pubsub: &testPublisher{}, - Database: db.Database{ - DB: jimmtest.PostgresDB(c, nil), - }, - Dialer: &jimmtest.Dialer{ - API: &jimmtest.API{ - AllModelWatcherNext_: func(_ context.Context, _ string) ([]jujuparams.Delta, error) { - cancel() - <-ctx.Done() - return nil, ctx.Err() - }, - ModelInfo_: func(_ context.Context, info *jujuparams.ModelInfo) error { - switch info.UUID { - default: - c.Errorf("unexpected model uuid: %s", info.UUID) - case "00000002-0000-0000-0000-000000000001": - } - info.Migration = &jujuparams.ModelMigrationStatus{Status: migration.ABORTDONE.String()} - return nil - }, - WatchAllModels_: func(ctx context.Context) (string, error) { - return "1234", nil - }, - }, - }, - } - env := jimmtest.ParseEnvironment(c, testFailedMigratingModelsWatcherEnv) - err := w.Database.Migrate(ctx, false) - c.Assert(err, qt.IsNil) - env.PopulateDB(c, w.Database) - ctl := env.Controllers[0].DBObject(c, w.Database) - - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - err := jimm.WatchController(w, ctx, &ctl) - checkIfContextCanceled(c, ctx, err) - }() - wg.Wait() - - modelInternalMigrated := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - } - err = w.Database.GetModel(context.Background(), &modelInternalMigrated) - c.Assert(err, qt.IsNil) - c.Assert(modelInternalMigrated.Controller.Name, qt.Equals, "controller-1") - c.Assert(modelInternalMigrated.Life, qt.Equals, constants.ALIVE.String()) - c.Assert(modelInternalMigrated.MigrationControllerID, qt.Equals, sql.NullInt32{}) -} - const testWatcherIgnoreDeltasForModelsFromIncorrectControllerEnv = `clouds: - name: test-cloud type: test-provider @@ -1178,7 +974,7 @@ func TestWatcherIgnoreDeltasForModelsFromIncorrectController(t *testing.T) { ModelUUID: "00000002-0000-0000-0000-000000000001", Name: "model-1", Owner: "alice@canonical.com", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: jujuparams.StatusInfo{ Current: "busy", }, diff --git a/internal/jimmtest/auth.go b/internal/jimmtest/auth.go index 4643f7336..787b1cb56 100644 --- a/internal/jimmtest/auth.go +++ b/internal/jimmtest/auth.go @@ -34,7 +34,10 @@ import ( ) const ( - JWTTestSecret = "test-secret" + // Note that these values are deliberately different to make sure we're not + // reusing/misusing them. + JWTTestSecret = "test-secret" + SessionStoreSecret = "another-test-secret" ) // A SimpleTester is a simple version of the test interface diff --git a/internal/jimmtest/cmp.go b/internal/jimmtest/cmp.go index 404bef2fb..6bdbe1554 100644 --- a/internal/jimmtest/cmp.go +++ b/internal/jimmtest/cmp.go @@ -45,7 +45,7 @@ var DBObjectEquals = qt.CmpEquals( cmpopts.IgnoreFields(dbmodel.CloudCredential{}, "CloudName", "OwnerIdentityName"), cmpopts.IgnoreFields(dbmodel.CloudRegion{}, "CloudName"), cmpopts.IgnoreFields(dbmodel.CloudRegionControllerPriority{}, "CloudRegionID", "ControllerID"), - cmpopts.IgnoreFields(dbmodel.Controller{}, "ID"), + cmpopts.IgnoreFields(dbmodel.Controller{}, "ID", "UpdatedAt", "CreatedAt"), cmpopts.IgnoreFields(dbmodel.Model{}, "ID", "CreatedAt", "UpdatedAt", "OwnerIdentityName", "ControllerID", "CloudRegionID", "CloudCredentialID"), ) diff --git a/internal/jimmtest/jimm_mock.go b/internal/jimmtest/jimm_mock.go index 244f2a8e5..c30222ba1 100644 --- a/internal/jimmtest/jimm_mock.go +++ b/internal/jimmtest/jimm_mock.go @@ -74,7 +74,7 @@ type JIMM struct { GrantServiceAccountAccess_ func(ctx context.Context, u *openfga.User, svcAccTag jimmnames.ServiceAccountTag, entities []string) error ImportModel_ func(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag, newOwner string) error IdentityModelDefaults_ func(ctx context.Context, user *dbmodel.Identity) (map[string]interface{}, error) - InitiateMigration_ func(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec, targetControllerID uint) (jujuparams.InitiateMigrationResult, error) + InitiateMigration_ func(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error) InitiateInternalMigration_ func(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetController string) (jujuparams.InitiateMigrationResult, error) ListApplicationOffers_ func(ctx context.Context, user *openfga.User, filters ...jujuparams.OfferFilter) ([]jujuparams.ApplicationOfferAdminDetailsV5, error) ListControllers_ func(ctx context.Context, user *openfga.User) ([]dbmodel.Controller, error) @@ -382,11 +382,11 @@ func (j *JIMM) ImportModel(ctx context.Context, user *openfga.User, controllerNa } return j.ImportModel_(ctx, user, controllerName, modelTag, newOwner) } -func (j *JIMM) InitiateMigration(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec, targetControllerID uint) (jujuparams.InitiateMigrationResult, error) { +func (j *JIMM) InitiateMigration(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error) { if j.InitiateMigration_ == nil { return jujuparams.InitiateMigrationResult{}, errors.E(errors.CodeNotImplemented) } - return j.InitiateMigration_(ctx, user, spec, targetControllerID) + return j.InitiateMigration_(ctx, user, spec) } func (j *JIMM) InitiateInternalMigration(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetController string) (jujuparams.InitiateMigrationResult, error) { if j.InitiateInternalMigration_ == nil { diff --git a/internal/jimmtest/store.go b/internal/jimmtest/store.go index f44fc0972..9ce783ef7 100644 --- a/internal/jimmtest/store.go +++ b/internal/jimmtest/store.go @@ -25,6 +25,7 @@ type InMemoryCredentialStore struct { privateKey []byte expiry time.Time oauthKey []byte + oauthSessionStoreSecret []byte controllerCredentials map[string]controllerCredentials cloudCredentialAttributes map[string]map[string]string } @@ -33,7 +34,8 @@ type InMemoryCredentialStore struct { // with some secrets/keys being populated. func NewInMemoryCredentialStore() *InMemoryCredentialStore { return &InMemoryCredentialStore{ - oauthKey: []byte(JWTTestSecret), + oauthKey: []byte(JWTTestSecret), + oauthSessionStoreSecret: []byte(SessionStoreSecret), } } @@ -193,6 +195,7 @@ func (s *InMemoryCredentialStore) CleanupOAuthSecrets(ctx context.Context) error defer s.mu.Unlock() s.oauthKey = nil + s.oauthSessionStoreSecret = nil return nil } @@ -221,3 +224,29 @@ func (s *InMemoryCredentialStore) PutOAuthSecret(ctx context.Context, raw []byte return nil } + +// GetOAuthSessionStoreSecret returns the current secret used to store session tokens. +func (s *InMemoryCredentialStore) GetOAuthSessionStoreSecret(ctx context.Context) ([]byte, error) { + s.mu.RLock() + defer s.mu.RUnlock() + + if s.oauthSessionStoreSecret == nil || len(s.oauthSessionStoreSecret) == 0 { + return nil, errors.E(errors.CodeNotFound) + } + + key := make([]byte, len(s.oauthSessionStoreSecret)) + copy(key, s.oauthSessionStoreSecret) + + return key, nil +} + +// PutOAuthSessionStoreSecret puts a secret into the credentials store for secure storage of session tokens. +func (s *InMemoryCredentialStore) PutOAuthSessionStoreSecret(ctx context.Context, raw []byte) error { + s.mu.Lock() + defer s.mu.Unlock() + + s.oauthSessionStoreSecret = make([]byte, len(raw)) + copy(s.oauthSessionStoreSecret, raw) + + return nil +} diff --git a/internal/jujuapi/access_control.go b/internal/jujuapi/access_control.go index 4ac451e2a..23388079c 100644 --- a/internal/jujuapi/access_control.go +++ b/internal/jujuapi/access_control.go @@ -242,10 +242,9 @@ func (r *controllerRoot) parseTuple(ctx context.Context, tuple apiparams.Relatio // ListRelationshipTuples returns a list of tuples matching the specified filter. func (r *controllerRoot) ListRelationshipTuples(ctx context.Context, req apiparams.ListRelationshipTuplesRequest) (apiparams.ListRelationshipTuplesResponse, error) { const op = errors.Op("jujuapi.ListRelationshipTuples") - var returnValue apiparams.ListRelationshipTuplesResponse if !r.user.JimmAdmin { - return returnValue, errors.E(op, errors.CodeUnauthorized, "unauthorized") + return apiparams.ListRelationshipTuplesResponse{}, errors.E(op, errors.CodeUnauthorized, "unauthorized") } key := &openfga.Tuple{} @@ -253,22 +252,25 @@ func (r *controllerRoot) ListRelationshipTuples(ctx context.Context, req apipara if req.Tuple.TargetObject != "" { key, err = r.parseTuple(ctx, req.Tuple) if err != nil { - return returnValue, errors.E(op, err) + return apiparams.ListRelationshipTuplesResponse{}, errors.E(op, err) } } responseTuples, ct, err := r.jimm.AuthorizationClient().ReadRelatedObjects(ctx, *key, req.PageSize, req.ContinuationToken) if err != nil { - return returnValue, errors.E(op, err) + return apiparams.ListRelationshipTuplesResponse{}, errors.E(op, err) } + errors := []string{} tuples := make([]apiparams.RelationshipTuple, len(responseTuples)) for i, t := range responseTuples { object, err := r.jimm.ToJAASTag(ctx, t.Object) if err != nil { - return returnValue, errors.E(op, err) + object = t.Object.String() + errors = append(errors, "failed to parse object: "+err.Error()) } target, err := r.jimm.ToJAASTag(ctx, t.Target) if err != nil { - return returnValue, errors.E(op, err) + target = t.Target.String() + errors = append(errors, "failed to parse target: "+err.Error()) } tuples[i] = apiparams.RelationshipTuple{ Object: object, @@ -279,5 +281,6 @@ func (r *controllerRoot) ListRelationshipTuples(ctx context.Context, req apipara return apiparams.ListRelationshipTuplesResponse{ Tuples: tuples, ContinuationToken: ct, + Errors: errors, }, nil } diff --git a/internal/jujuapi/access_control_test.go b/internal/jujuapi/access_control_test.go index 87ab92ddb..b5327f9af 100644 --- a/internal/jujuapi/access_control_test.go +++ b/internal/jujuapi/access_control_test.go @@ -11,12 +11,12 @@ import ( petname "github.com/dustinkirkland/golang-petname" "github.com/google/uuid" "github.com/juju/juju/core/crossmodel" + "github.com/juju/juju/state" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" "github.com/canonical/jimm/api" apiparams "github.com/canonical/jimm/api/params" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/jimmtest" "github.com/canonical/jimm/internal/jujuapi" @@ -864,6 +864,7 @@ func (s *accessControlSuite) TestListRelationshipTuples(c *gc.C) { c.Assert(err, jc.ErrorIsNil) // first three tuples created during setup test c.Assert(response.Tuples[12:], jc.DeepEquals, tuples) + c.Assert(len(response.Errors), gc.Equals, 0) response, err = client.ListRelationshipTuples(&apiparams.ListRelationshipTuplesRequest{ Tuple: apiparams.RelationshipTuple{ @@ -872,7 +873,85 @@ func (s *accessControlSuite) TestListRelationshipTuples(c *gc.C) { }) c.Assert(err, jc.ErrorIsNil) c.Assert(response.Tuples, jc.DeepEquals, []apiparams.RelationshipTuple{tuples[3]}) + c.Assert(len(response.Errors), gc.Equals, 0) +} + +func (s *accessControlSuite) TestListRelationshipTuplesAfterDeletingGroup(c *gc.C) { + ctx := context.Background() + user, _, controller, model, applicationOffer, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) + defer closeClient() + + err := client.AddGroup(&apiparams.AddGroupRequest{Name: "yellow"}) + c.Assert(err, jc.ErrorIsNil) + err = client.AddGroup(&apiparams.AddGroupRequest{Name: "orange"}) + c.Assert(err, jc.ErrorIsNil) + + tuples := []apiparams.RelationshipTuple{{ + Object: "group-orange#member", + Relation: "member", + TargetObject: "group-yellow", + }, { + Object: "user-" + user.Name, + Relation: "member", + TargetObject: "group-orange", + }, { + Object: "group-yellow#member", + Relation: "administrator", + TargetObject: "controller-" + controller.Name, + }, { + Object: "group-orange#member", + Relation: "administrator", + TargetObject: "applicationoffer-" + controller.Name + ":" + user.Name + "/" + model.Name + "." + applicationOffer.Name, + }} + + err = client.AddRelation(&apiparams.AddRelationRequest{Tuples: tuples}) + c.Assert(err, jc.ErrorIsNil) + err = client.RemoveGroup(&apiparams.RemoveGroupRequest{Name: "yellow"}) + c.Assert(err, jc.ErrorIsNil) + + response, err := client.ListRelationshipTuples(&apiparams.ListRelationshipTuplesRequest{}) + c.Assert(err, jc.ErrorIsNil) + // Create a new slice of tuples excluding the ones we expect to be deleted. + newTuples := []apiparams.RelationshipTuple{tuples[1], tuples[3]} + // first three tuples created during setup test + c.Assert(response.Tuples[12:], jc.DeepEquals, newTuples) + c.Assert(len(response.Errors), gc.Equals, 0) +} + +func (s *accessControlSuite) TestListRelationshipTuplesWithMissingGroups(c *gc.C) { + ctx := context.Background() + _, _, _, _, _, _, _, client, closeClient := createTestControllerEnvironment(ctx, c, s) + defer closeClient() + + err := client.AddGroup(&apiparams.AddGroupRequest{Name: "yellow"}) + c.Assert(err, jc.ErrorIsNil) + err = client.AddGroup(&apiparams.AddGroupRequest{Name: "orange"}) + c.Assert(err, jc.ErrorIsNil) + + tuples := []apiparams.RelationshipTuple{{ + Object: "group-orange#member", + Relation: "member", + TargetObject: "group-yellow", + }} + + err = client.AddRelation(&apiparams.AddRelationRequest{Tuples: tuples}) + c.Assert(err, jc.ErrorIsNil) + + // Delete a group without going through the API. + group := &dbmodel.GroupEntry{Name: "yellow"} + err = s.JIMM.DB().GetGroup(ctx, group) + c.Assert(err, jc.ErrorIsNil) + err = s.JIMM.DB().RemoveGroup(ctx, group) + c.Assert(err, jc.ErrorIsNil) + + response, err := client.ListRelationshipTuples(&apiparams.ListRelationshipTuplesRequest{}) + c.Assert(err, jc.ErrorIsNil) + tupleWithoutDBEntry := tuples[0] + tupleWithoutDBEntry.TargetObject = "group:" + group.UUID + // first three tuples created during setup test + c.Assert(response.Tuples[12], gc.Equals, tupleWithoutDBEntry) + c.Assert(response.Errors, gc.DeepEquals, []string{"failed to parse target: failed to fetch group information: " + group.UUID}) } func (s *accessControlSuite) TestCheckRelationAsNonAdmin(c *gc.C) { @@ -1393,7 +1472,7 @@ func createTestControllerEnvironment(ctx context.Context, c *gc.C, s *accessCont ControllerID: controller.ID, CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred.ID, - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: sql.NullTime{ diff --git a/internal/jujuapi/controller.go b/internal/jujuapi/controller.go index 6af5964b0..96a00cd5a 100644 --- a/internal/jujuapi/controller.go +++ b/internal/jujuapi/controller.go @@ -295,7 +295,7 @@ func (r *controllerRoot) InitiateMigration(ctx context.Context, args jujuparams. results := make([]jujuparams.InitiateMigrationResult, len(args.Specs)) for i, spec := range args.Specs { - result, err := r.jimm.InitiateMigration(ctx, r.user, spec, 0) + result, err := r.jimm.InitiateMigration(ctx, r.user, spec) if err != nil { result.Error = mapError(errors.E(op, err)) } diff --git a/internal/jujuapi/controller_test.go b/internal/jujuapi/controller_test.go index 56833cd73..bb9c290e0 100644 --- a/internal/jujuapi/controller_test.go +++ b/internal/jujuapi/controller_test.go @@ -16,12 +16,12 @@ import ( "github.com/juju/juju/controller" "github.com/juju/juju/core/life" jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/juju/state" jujuversion "github.com/juju/juju/version" "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/errors" "github.com/canonical/jimm/internal/jimmtest" "github.com/canonical/jimm/internal/jujuapi" @@ -123,7 +123,7 @@ func (s *controllerSuite) TestModelStatus(c *gc.C) { c.Assert(models, gc.HasLen, 2) c.Check(models[0], jc.DeepEquals, base.ModelStatus{ UUID: s.Model.UUID.String, - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Owner: "bob@canonical.com", TotalMachineCount: 0, CoreCount: 0, @@ -329,13 +329,13 @@ func TestInitiateMigration(t *testing.T) { tests := []struct { about string - initiateMigration func(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec, targetControllerID uint) (jujuparams.InitiateMigrationResult, error) + initiateMigration func(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error) args jujuparams.InitiateMigrationArgs expectedError string expectedResult jujuparams.InitiateMigrationResults }{{ about: "model migration initiated successfully", - initiateMigration: func(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec, targetControllerID uint) (jujuparams.InitiateMigrationResult, error) { + initiateMigration: func(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error) { return jujuparams.InitiateMigrationResult{ ModelTag: mt.String(), MigrationId: migrationID, @@ -354,7 +354,7 @@ func TestInitiateMigration(t *testing.T) { }, }, { about: "controller returns an error", - initiateMigration: func(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec, targetControllerID uint) (jujuparams.InitiateMigrationResult, error) { + initiateMigration: func(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error) { return jujuparams.InitiateMigrationResult{}, errors.E("a silly error") }, args: jujuparams.InitiateMigrationArgs{ diff --git a/internal/jujuapi/controllerroot.go b/internal/jujuapi/controllerroot.go index 24207c542..03652a542 100644 --- a/internal/jujuapi/controllerroot.go +++ b/internal/jujuapi/controllerroot.go @@ -73,7 +73,7 @@ type JIMM interface { IdentityModelDefaults(ctx context.Context, user *dbmodel.Identity) (map[string]interface{}, error) ImportModel(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag, newOwner string) error InitiateInternalMigration(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetController string) (jujuparams.InitiateMigrationResult, error) - InitiateMigration(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec, targetControllerID uint) (jujuparams.InitiateMigrationResult, error) + InitiateMigration(ctx context.Context, user *openfga.User, spec jujuparams.MigrationSpec) (jujuparams.InitiateMigrationResult, error) ListApplicationOffers(ctx context.Context, user *openfga.User, filters ...jujuparams.OfferFilter) ([]jujuparams.ApplicationOfferAdminDetailsV5, error) ListGroups(ctx context.Context, user *openfga.User) ([]dbmodel.GroupEntry, error) ModelDefaultsForCloud(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag) (jujuparams.ModelDefaultsResult, error) diff --git a/internal/jujuapi/jimm.go b/internal/jujuapi/jimm.go index 60be3cc56..75a15e7f0 100644 --- a/internal/jujuapi/jimm.go +++ b/internal/jujuapi/jimm.go @@ -180,6 +180,7 @@ func (r *controllerRoot) AddController(ctx context.Context, req apiparams.AddCon CACertificate: req.CACertificate, AdminIdentityName: req.Username, AdminPassword: req.Password, + TLSHostname: req.TLSHostname, } nphps, err := network.ParseProviderHostPorts(req.APIAddresses...) if err != nil { diff --git a/internal/jujuapi/jimm_test.go b/internal/jujuapi/jimm_test.go index b875c6677..c626af755 100644 --- a/internal/jujuapi/jimm_test.go +++ b/internal/jujuapi/jimm_test.go @@ -186,6 +186,69 @@ func (s *jimmSuite) TestAddController(c *gc.C) { c.Assert(jujuparams.IsBadRequest(err), gc.Equals, true) } +func (s *jimmSuite) TestRemoveAndAddController(c *gc.C) { + conn := s.open(c, nil, "alice") + defer conn.Close() + client := api.NewClient(conn) + + info := s.APIInfo(c) + + acr := apiparams.AddControllerRequest{ + UUID: info.ControllerUUID, + Name: "controller-2", + APIAddresses: info.Addrs, + CACertificate: info.CACert, + Username: info.Tag.Id(), + Password: info.Password, + } + + ci, err := client.AddController(&acr) + c.Assert(err, gc.Equals, nil) + _, err = client.RemoveController(&apiparams.RemoveControllerRequest{Name: acr.Name, Force: true}) + c.Assert(err, gc.Equals, nil) + ciNew, err := client.AddController(&acr) + c.Assert(err, gc.Equals, nil) + c.Assert(ci, gc.DeepEquals, ciNew) + +} + +func (s *jimmSuite) TestAddControllerCustomTLSHostname(c *gc.C) { + conn := s.open(c, nil, "alice") + defer conn.Close() + client := api.NewClient(conn) + + info := s.APIInfo(c) + + acr := apiparams.AddControllerRequest{ + UUID: info.ControllerUUID, + Name: "controller-2", + APIAddresses: info.Addrs, + CACertificate: info.CACert, + Username: info.Tag.Id(), + Password: info.Password, + TLSHostname: "foo", + } + + _, err := client.AddController(&acr) + c.Assert(err, gc.ErrorMatches, "failed to dial the controller") + acr.TLSHostname = "juju-apiserver" + ci, err := client.AddController(&acr) + c.Assert(err, gc.IsNil) + c.Assert(ci, jc.DeepEquals, apiparams.ControllerInfo{ + Name: "controller-2", + UUID: info.ControllerUUID, + APIAddresses: info.Addrs, + CACertificate: info.CACert, + CloudTag: names.NewCloudTag(jimmtest.TestCloudName).String(), + CloudRegion: jimmtest.TestCloudRegionName, + AgentVersion: s.Model.Controller.AgentVersion, + Status: jujuparams.EntityStatus{ + Status: "available", + }, + }) + +} + func (s *jimmSuite) TestRemoveController(c *gc.C) { conn := s.open(c, nil, "alice") defer conn.Close() diff --git a/internal/jujuapi/modelmanager_test.go b/internal/jujuapi/modelmanager_test.go index d65931031..3a5bfb87c 100644 --- a/internal/jujuapi/modelmanager_test.go +++ b/internal/jujuapi/modelmanager_test.go @@ -26,7 +26,6 @@ import ( "github.com/juju/utils/v2" gc "gopkg.in/check.v1" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/jimmtest" "github.com/canonical/jimm/internal/kubetest" @@ -72,7 +71,7 @@ func (s *modelManagerSuite) TestListModelSummaries(c *gc.C) { CloudRegion: jimmtest.TestCloudRegionName, CloudCredential: jimmtest.TestCloudName + "/bob@canonical.com/cred", Owner: "bob@canonical.com", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: base.Status{ Status: status.Available, Data: map[string]interface{}{}, @@ -103,7 +102,7 @@ func (s *modelManagerSuite) TestListModelSummaries(c *gc.C) { CloudRegion: jimmtest.TestCloudRegionName, CloudCredential: jimmtest.TestCloudName + "/charlie@canonical.com/cred", Owner: "charlie@canonical.com", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: base.Status{ Status: status.Available, Data: map[string]interface{}{}, @@ -180,7 +179,7 @@ func (s *modelManagerSuite) TestListModelSummariesWithoutControllerUUIDMasking(c CloudRegion: jimmtest.TestCloudRegionName, CloudCredential: jimmtest.TestCloudName + "/bob@canonical.com/cred", Owner: "bob@canonical.com", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: base.Status{ Status: status.Available, Data: map[string]interface{}{}, @@ -211,7 +210,7 @@ func (s *modelManagerSuite) TestListModelSummariesWithoutControllerUUIDMasking(c CloudRegion: jimmtest.TestCloudRegionName, CloudCredential: jimmtest.TestCloudName + "/charlie@canonical.com/cred", Owner: "charlie@canonical.com", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: base.Status{ Status: status.Available, Data: map[string]interface{}{}, @@ -1405,7 +1404,7 @@ func (s *caasModelManagerSuite) TestListCAASModelSummaries(c *gc.C) { CloudRegion: "default", CloudCredential: s.cred.Id(), Owner: "bob@canonical.com", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: base.Status{ Status: status.Available, Data: map[string]interface{}{}, @@ -1437,7 +1436,7 @@ func (s *caasModelManagerSuite) TestListCAASModelSummaries(c *gc.C) { CloudRegion: jimmtest.TestCloudRegionName, CloudCredential: jimmtest.TestCloudName + "/bob@canonical.com/cred", Owner: "bob@canonical.com", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: base.Status{ Status: status.Available, Data: map[string]interface{}{}, @@ -1459,7 +1458,7 @@ func (s *caasModelManagerSuite) TestListCAASModelSummaries(c *gc.C) { CloudRegion: jimmtest.TestCloudRegionName, CloudCredential: jimmtest.TestCloudName + "/charlie@canonical.com/cred", Owner: "charlie@canonical.com", - Life: life.Value(constants.ALIVE.String()), + Life: life.Value(state.Alive.String()), Status: base.Status{ Status: status.Available, Data: map[string]interface{}{}, diff --git a/internal/jujuclient/modelmanager_test.go b/internal/jujuclient/modelmanager_test.go index fddd319d9..a745eb016 100644 --- a/internal/jujuclient/modelmanager_test.go +++ b/internal/jujuclient/modelmanager_test.go @@ -10,12 +10,12 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/juju/juju/core/life" jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/juju/state" "github.com/juju/names/v5" jc "github.com/juju/testing/checkers" "github.com/juju/utils/v2" gc "gopkg.in/check.v1" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/jimmtest" ) @@ -39,7 +39,7 @@ func (s *modelmanagerSuite) TestCreateModel(c *gc.C) { c.Check(info.CloudTag, gc.Equals, names.NewCloudTag(jimmtest.TestCloudName).String()) c.Check(info.CloudRegion, gc.Equals, jimmtest.TestCloudRegionName) c.Check(info.DefaultSeries, gc.Equals, "jammy") - c.Check(string(info.Life), gc.Equals, constants.ALIVE.String()) + c.Check(string(info.Life), gc.Equals, state.Alive.String()) c.Check(string(info.Status.Status), gc.Equals, "available") c.Check(info.Status.Data, gc.IsNil) c.Check(info.Status.Since.After(time.Now().Add(-10*time.Second)), gc.Equals, true) diff --git a/internal/openfga/export_test.go b/internal/openfga/export_test.go index 47d6be97e..e0c865549 100644 --- a/internal/openfga/export_test.go +++ b/internal/openfga/export_test.go @@ -2,7 +2,17 @@ package openfga -import "context" +import ( + "context" + + ofganames "github.com/canonical/jimm/internal/openfga/names" +) + +// This is just for exporting the private function for testing. Since the private +// function is a generic one, we cannot use a simple file-scoped "var" statement. +func UnsetMultipleResourceAccesses[T ofganames.ResourceTagger](ctx context.Context, user *User, resource T, relations []Relation, pageSize int32) error { + return unsetMultipleResourceAccesses(ctx, user, resource, relations, pageSize) +} func (o *OFGAClient) RemoveTuples(ctx context.Context, tuple Tuple) error { return o.removeTuples(ctx, tuple) diff --git a/internal/openfga/user.go b/internal/openfga/user.go index c0a780942..40255f2ab 100644 --- a/internal/openfga/user.go +++ b/internal/openfga/user.go @@ -206,7 +206,7 @@ func (u *User) SetModelAccess(ctx context.Context, resource names.ModelTag, rela // UnsetModelAccess removes direct relations between the user and the model. // Note that the action is idempotent (i.e., does not return error if the relation does not exist). func (u *User) UnsetModelAccess(ctx context.Context, resource names.ModelTag, relations ...Relation) error { - return unsetMultipleResourceAccesses(ctx, u, resource, relations) + return unsetMultipleResourceAccesses(ctx, u, resource, relations, 0) } // SetControllerAccess adds a direct relation between the user and the controller. @@ -230,7 +230,7 @@ func (u *User) SetCloudAccess(ctx context.Context, resource names.CloudTag, rela // UnsetCloudAccess removes direct relations between the user and the cloud. // Note that the action is idempotent (i.e., does not return error if the relation does not exist). func (u *User) UnsetCloudAccess(ctx context.Context, resource names.CloudTag, relations ...Relation) error { - return unsetMultipleResourceAccesses(ctx, u, resource, relations) + return unsetMultipleResourceAccesses(ctx, u, resource, relations, 0) } // SetApplicationOfferAccess adds a direct relation between the user and the application offer. @@ -334,7 +334,7 @@ func IsAdministrator[T administratorT](ctx context.Context, u *User, resource T) return false, errors.E(err) } if isAdmin { - zapctx.Info( + zapctx.Debug( ctx, "user is resource administrator", zap.String("user", u.Tag().String()), @@ -364,9 +364,14 @@ func setResourceAccess[T ofganames.ResourceTagger](ctx context.Context, user *Us return nil } -// unsetMultipleResourceAccesses deletes relations that correspond to the requested resource access, atomically. -// Note that the action is idempotent (i.e., does not return error if any of the relations does not exist). -func unsetMultipleResourceAccesses[T ofganames.ResourceTagger](ctx context.Context, user *User, resource T, relations []Relation) error { +// unsetMultipleResourceAccesses deletes relations that correspond to the +// requested resource access, atomically. The pageSize argument determines the +// read requests chunk size, and can be set to zero to opt to OpenFGA client +// defaults. +// +// Note that the action is idempotent (i.e., does not return error if any of the +// relations does not exist). +func unsetMultipleResourceAccesses[T ofganames.ResourceTagger](ctx context.Context, user *User, resource T, relations []Relation, pageSize int32) error { tupleObject := ofganames.ConvertTag(user.ResourceTag()) tupleTarget := ofganames.ConvertTag(resource) @@ -376,7 +381,7 @@ func unsetMultipleResourceAccesses[T ofganames.ResourceTagger](ctx context.Conte timestampedTuples, continuationToken, err := user.client.cofgaClient.FindMatchingTuples(ctx, Tuple{ Object: tupleObject, Target: tupleTarget, - }, 0, lastContinuationToken) + }, pageSize, lastContinuationToken) if err != nil { return errors.E(err, "failed to retrieve existing relations") @@ -386,7 +391,7 @@ func unsetMultipleResourceAccesses[T ofganames.ResourceTagger](ctx context.Conte existingRelations[timestampedTuple.Tuple.Relation] = nil } - if continuationToken == lastContinuationToken { + if continuationToken == "" { break } lastContinuationToken = continuationToken diff --git a/internal/openfga/user_test.go b/internal/openfga/user_test.go index a94691d9a..860977f05 100644 --- a/internal/openfga/user_test.go +++ b/internal/openfga/user_test.go @@ -576,3 +576,81 @@ func (s *userTestSuite) TestListApplicationOffers(c *gc.C) { sort.Strings(offerUUIDs) c.Assert(offerUUIDs, gc.DeepEquals, wantUUIDs) } + +func (s *userTestSuite) TestUnsetMultipleResourceAccesses(c *gc.C) { + ctx := context.Background() + + tests := []struct { + name string + pageSize int32 + }{{ + name: "pageSize: 0 (OpenFGA default)", + pageSize: 0, + }, { + name: "pageSize: 100 (OpenFGA max)", + pageSize: 100, + }, { + name: "pageSize: 1", + pageSize: 1, + }, { + name: "pageSize: 2", + pageSize: 2, + }, { + name: "pageSize: 3", + pageSize: 3, + }, { + name: "pageSize: 4", + pageSize: 4, + }} + + for _, tt := range tests { + modelUUID, err := uuid.NewRandom() + c.Assert(err, gc.IsNil) + model := names.NewModelTag(modelUUID.String()) + + adam := names.NewUserTag("adam") + + adamIdentity, err := dbmodel.NewIdentity("adam") + c.Assert(err, gc.IsNil) + + adamUser := openfga.NewUser(adamIdentity, s.ofgaClient) + + // Note that the total number of tuples in OpenFGA actually has no + // effect here, because the `unsetMultipleResourceAccesses` function + // queries for tuples that have a specific object and target. So, the + // returned tuples are just a few. That's why we've used user-to-model + // tuples in this test because they have the highest number of possible + // relations (i.e., reader, writer, and administrator). + tuples := []openfga.Tuple{{ + Object: ofganames.ConvertTag(adam), + Relation: ofganames.ReaderRelation, + Target: ofganames.ConvertTag(model), + }, { + Object: ofganames.ConvertTag(adam), + Relation: ofganames.WriterRelation, + Target: ofganames.ConvertTag(model), + }, { + Object: ofganames.ConvertTag(adam), + Relation: ofganames.AdministratorRelation, + Target: ofganames.ConvertTag(model), + }} + + err = s.ofgaClient.AddRelation(ctx, tuples...) + c.Assert(err, gc.IsNil) + + err = openfga.UnsetMultipleResourceAccesses( + ctx, adamUser, model, + []openfga.Relation{ + ofganames.ReaderRelation, + ofganames.WriterRelation, + ofganames.AdministratorRelation, + }, + tt.pageSize, + ) + c.Assert(err, gc.IsNil) + + retrieved, _, err := s.cofgaClient.FindMatchingTuples(ctx, openfga.Tuple{Target: ofganames.ConvertTag(model)}, 0, "") + c.Assert(err, gc.IsNil) + c.Assert(retrieved, gc.HasLen, 0) + } +} diff --git a/internal/rpc/dial.go b/internal/rpc/dial.go index e043974a8..5994f3412 100644 --- a/internal/rpc/dial.go +++ b/internal/rpc/dial.go @@ -63,7 +63,8 @@ func Dial(ctx context.Context, ctl *dbmodel.Controller, modelTag names.ModelTag, zapctx.Warn(ctx, "no CA certificates added") } tlsConfig = &tls.Config{ - RootCAs: cp, + RootCAs: cp, + ServerName: ctl.TLSHostname, } } dialer := Dialer{ diff --git a/internal/vault/vault.go b/internal/vault/vault.go index 52698b578..4393ea635 100644 --- a/internal/vault/vault.go +++ b/internal/vault/vault.go @@ -27,10 +27,11 @@ const ( ) const ( - jwksKey = "jwks" - jwksExpiryKey = "jwks-expiry" - jwksPrivateKey = "jwks-private" - oAuthSecretKey = "oauth-secret" + jwksKey = "jwks" + jwksExpiryKey = "jwks-expiry" + jwksPrivateKey = "jwks-private" + oAuthSecretKey = "oauth-secret" + oAuthSessionStoreSecretKey = "oauth-session-store-secret" ) // A VaultStore stores cloud credential attributes and @@ -392,9 +393,18 @@ func (s *VaultStore) CleanupOAuthSecrets(ctx context.Context) error { if err := client.KVv2(s.KVPath).Delete(ctx, s.GetOAuthSecretPath()); err != nil { return errors.E(op, err) } + if err := client.KVv2(s.KVPath).Delete(ctx, s.GetOAuthSessionStoreSecretPath()); err != nil { + return errors.E(op, err) + } return nil } +// GetOAuthSecretsBasePath returns a hardcoded suffixed vault path (dependent on +// the initial KVPath) representing the base path for OAuth related secrets. +func (s *VaultStore) GetOAuthSecretsBasePath() string { + return path.Join("creds", "oauth") +} + // GetOAuthSecret returns the current HS256 (symmetric encryption) secret used to sign OAuth session tokens. func (s *VaultStore) GetOAuthSecret(ctx context.Context) ([]byte, error) { const op = errors.Op("vault.GetOAuthSecret") @@ -455,7 +465,70 @@ func (s *VaultStore) PutOAuthSecret(ctx context.Context, raw []byte) error { // GetOAuthSecretPath returns a hardcoded suffixed vault path (dependent on // the initial KVPath) to the OAuth JWK location. func (s *VaultStore) GetOAuthSecretPath() string { - return path.Join("creds", "oauth", "key") + return path.Join(s.GetOAuthSecretsBasePath(), "key") +} + +// GetOAuthSessionStoreSecret returns the current secret used to store session tokens. +func (s *VaultStore) GetOAuthSessionStoreSecret(ctx context.Context) ([]byte, error) { + const op = errors.Op("vault.GetOAuthSessionStoreSecret") + + client, err := s.client(ctx) + if err != nil { + return nil, errors.E(op, err) + } + + secret, err := client.KVv2(s.KVPath).Get(ctx, s.GetOAuthSessionStoreSecretPath()) + if err != nil && goerr.Unwrap(err) != api.ErrSecretNotFound { + return nil, errors.E(op, err) + } + + if secret == nil || secret.Data == nil { + msg := "no OAuth session store secret exists" + zapctx.Debug(ctx, msg) + return nil, errors.E(op, errors.CodeNotFound, msg) + } + + raw, ok := secret.Data[oAuthSessionStoreSecretKey] + if !ok { + msg := "nil OAuth session store secret data" + zapctx.Debug(ctx, msg) + return nil, errors.E(op, errors.CodeNotFound, msg) + } + + keyPemB64, ok := raw.(string) + if !ok { + zapctx.Debug(ctx, "oauth session store secret is not a string") + return nil, errors.E(op, errors.CodeNotFound, "oauth session store secret not found") + } + + keyPem, err := base64.StdEncoding.DecodeString(keyPemB64) + if err != nil { + return nil, errors.E(op, err) + } + + return keyPem, nil +} + +// PutOAuthSessionStoreSecret puts a secret into the credentials store for secure storage of session tokens. +func (s *VaultStore) PutOAuthSessionStoreSecret(ctx context.Context, raw []byte) error { + const op = errors.Op("vault.PutOAuthSessionStoreSecret") + + client, err := s.client(ctx) + if err != nil { + return errors.E(op, err) + } + + oAuthSecretData := map[string]interface{}{oAuthSessionStoreSecretKey: raw} + if _, err := client.KVv2(s.KVPath).Put(ctx, s.GetOAuthSessionStoreSecretPath(), oAuthSecretData); err != nil { + return errors.E(op, err) + } + return nil +} + +// GetOAuthSessionStoreSecretPath returns a hardcoded suffixed vault path +// (dependent on the initial KVPath) to the OAuth session storage secret. +func (s *VaultStore) GetOAuthSessionStoreSecretPath() string { + return path.Join(s.GetOAuthSecretsBasePath(), "session-store") } // deleteControllerCredentials removes the credentials associated with the controller in diff --git a/internal/vault/vault_test.go b/internal/vault/vault_test.go index 2594258e8..d74e959fa 100644 --- a/internal/vault/vault_test.go +++ b/internal/vault/vault_test.go @@ -225,3 +225,44 @@ func TestGetOAuthSecretFailsIfNotFound(t *testing.T) { c.Assert(err, qt.ErrorMatches, "no OAuth key exists") c.Assert(retrieved, qt.IsNil) } + +func TestGetAndPutOAuthSessionStoreSecret(t *testing.T) { + c := qt.New(t) + ctx := context.Background() + store := newStore(c) + + // We didn't use a pre-defined/constant key here because in that case we had + // to make sure there's nothing left from last test runs in Vault. + key := []byte(uuid.NewString()) // A random UUID as key + err := store.PutOAuthSessionStoreSecret(ctx, key) + c.Assert(err, qt.IsNil) + retrievedKey, err := store.GetOAuthSessionStoreSecret(ctx) + c.Assert(err, qt.IsNil) + c.Assert(retrievedKey, qt.DeepEquals, key) +} + +func TestGetOAuthSessionStoreSecretFailsIfDataIsNil(t *testing.T) { + c := qt.New(t) + ctx := context.Background() + store := newStore(c) + + err := store.PutOAuthSessionStoreSecret(ctx, nil) + c.Assert(err, qt.IsNil) + + retrieved, err := store.GetOAuthSessionStoreSecret(ctx) + c.Assert(err, qt.ErrorMatches, "oauth session store secret not found") + c.Assert(retrieved, qt.IsNil) +} + +func TestGetOAuthSessionStoreSecretFailsIfNotFound(t *testing.T) { + c := qt.New(t) + ctx := context.Background() + store := newStore(c) + + err := store.CleanupOAuthSecrets(ctx) + c.Assert(err, qt.IsNil) + + retrieved, err := store.GetOAuthSessionStoreSecret(ctx) + c.Assert(err, qt.ErrorMatches, "no OAuth session store secret exists") + c.Assert(retrieved, qt.IsNil) +} diff --git a/local/README.md b/local/README.md index c43bc8c9b..c97ffca6e 100644 --- a/local/README.md +++ b/local/README.md @@ -6,8 +6,8 @@ used for integration testing within the JIMM test suite. # Starting the environment 1. Ensure you have `make` installed `sudo apt install make` -2. Check for system dependecies with `make sys-deps` this will inform you of any missing dependies and how to install them. -3. Set up necessary prequisites with `make dev-env-setup` +2. Check for system dependencies with `make sysdeps` this will inform you of any missing dependencies and how to install them. +3. Set up necessary prerequisites with `make dev-env-setup` 4. Start the dev env `make dev-env`, going forward you can skip steps 1-3. 5. To teardown the dev env `make dev-env-cleanup` @@ -47,7 +47,7 @@ The `request name` represents the literal WS endpoint, i.e., `API = /api`. 1. The following commands might need to be run to work around an [LXC networking issue](https://github.com/docker/for-linux/issues/103#issuecomment-383607773): `sudo iptables -F FORWARD && sudo iptables -P FORWARD ACCEPT`. -2. Install Juju: `sudo snap install juju --channel=3.3/stable` (minimum Juju version is `3.3`). +2. Install Juju: `sudo snap install juju --channel=3.5/stable` (minimum Juju version is `3.5`). 3. Install JQ: `sudo snap install jq`. ## All-In-One scripts diff --git a/local/seed_db/main.go b/local/seed_db/main.go index 1eb5da824..24d5a3a12 100644 --- a/local/seed_db/main.go +++ b/local/seed_db/main.go @@ -7,13 +7,13 @@ import ( "os" "time" - "github.com/canonical/jimm/internal/constants" "github.com/canonical/jimm/internal/db" "github.com/canonical/jimm/internal/dbmodel" "github.com/canonical/jimm/internal/logger" petname "github.com/dustinkirkland/golang-petname" "github.com/google/uuid" "github.com/juju/juju/core/crossmodel" + "github.com/juju/juju/state" "gorm.io/driver/postgres" "gorm.io/gorm" ) @@ -102,7 +102,7 @@ func main() { ControllerID: controller.ID, CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred.ID, - Life: constants.ALIVE.String(), + Life: state.Alive.String(), Status: dbmodel.Status{ Status: "available", Since: sql.NullTime{ diff --git a/service.go b/service.go index 62d9c15fd..4148e8deb 100644 --- a/service.go +++ b/service.go @@ -5,7 +5,6 @@ package jimm import ( "context" "crypto/rand" - "database/sql" "net/http" "net/url" "strconv" @@ -180,7 +179,8 @@ type Params struct { type Service struct { jimm jimm.JIMM - mux *chi.Mux + mux *chi.Mux + cleanups []func() error } func (s *Service) JIMM() *jimm.JIMM { @@ -225,6 +225,22 @@ func (s *Service) StartJWKSRotator(ctx context.Context, checkRotateRequired <-ch return s.jimm.JWKService.StartJWKSRotator(ctx, checkRotateRequired, initialRotateRequiredTime) } +// Cleanup cleans up resources that need to be released on shutdown. +func (s *Service) Cleanup() { + // Iterating over clean up function in reverse-order to avoid early clean ups. + for i := len(s.cleanups) - 1; i >= 0; i-- { + f := s.cleanups[i] + if err := f(); err != nil { + zapctx.Error(context.Background(), "cleanup failed", zap.Error(err)) + } + } +} + +// AddCleanup adds a clean up function to be run at service shutdown. +func (s *Service) AddCleanup(f func() error) { + s.cleanups = append(s.cleanups, f) +} + // NewService creates a new Service using the given params. func NewService(ctx context.Context, p Params) (*Service, error) { const op = errors.Op("NewService") @@ -256,19 +272,6 @@ func NewService(ctx context.Context, p Params) (*Service, error) { if err := s.jimm.Database.Migrate(ctx, false); err != nil { return nil, errors.E(op, err) } - sqlDb, err := s.jimm.Database.DB.DB() - if err != nil { - return nil, errors.E(op, err) - } - - // Setup browser session store - sessionStore, err := setupSessionStore(sqlDb, "secret-key-todo") - if err != nil { - return nil, errors.E(op, err) - } - - // Cleanup expired session every 30 minutes - defer sessionStore.StopCleanup(sessionStore.Cleanup(time.Minute * 30)) if p.AuditLogRetentionPeriodInDays != "" { period, err := strconv.Atoi(p.AuditLogRetentionPeriodInDays) @@ -292,6 +295,15 @@ func NewService(ctx context.Context, p Params) (*Service, error) { return nil, errors.E(op, err, "failed to ensure controller admins") } + if err := s.setupCredentialStore(ctx, p); err != nil { + return nil, errors.E(op, err) + } + + sessionStore, err := s.setupSessionStore(ctx) + if err != nil { + return nil, errors.E(op, err) + } + redirectUrl := p.PublicDNSName + jimmhttp.AuthResourceBasePath + jimmhttp.CallbackEndpoint if !strings.HasPrefix(redirectUrl, "https://") || !strings.HasPrefix(redirectUrl, "http://") { redirectUrl = "https://" + redirectUrl @@ -317,10 +329,6 @@ func NewService(ctx context.Context, p Params) (*Service, error) { return nil, errors.E(op, err, "failed to setup authentication service") } - if err := s.setupCredentialStore(ctx, p); err != nil { - return nil, errors.E(op, err) - } - if p.JWTExpiryDuration == 0 { p.JWTExpiryDuration = 24 * time.Hour } @@ -360,19 +368,25 @@ func NewService(ctx context.Context, p Params) (*Service, error) { "/.well-known", wellknownapi.NewWellKnownHandler(s.jimm.CredentialStore), ) - oauthHandler, err := jimmhttp.NewOAuthHandler(jimmhttp.OAuthHandlerParams{ - Authenticator: authSvc, - DashboardFinalRedirectURL: p.DashboardFinalRedirectURL, - SecureCookies: p.SecureSessionCookies, - }) - if err != nil { - zapctx.Error(ctx, "failed to setup authentication handler", zap.Error(err)) - return nil, errors.E(op, err, "failed to setup authentication handler") + + if p.DashboardFinalRedirectURL == "" { + zapctx.Warn(ctx, "OAuth handler not enabled, due to unset dashboard redirect URL") + } else { + oauthHandler, err := jimmhttp.NewOAuthHandler(jimmhttp.OAuthHandlerParams{ + Authenticator: authSvc, + DashboardFinalRedirectURL: p.DashboardFinalRedirectURL, + SecureCookies: p.SecureSessionCookies, + }) + if err != nil { + zapctx.Error(ctx, "failed to setup authentication handler", zap.Error(err)) + return nil, errors.E(op, err, "failed to setup authentication handler") + } + mountHandler( + jimmhttp.AuthResourceBasePath, + oauthHandler, + ) } - mountHandler( - jimmhttp.AuthResourceBasePath, - oauthHandler, - ) + macaroonDischarger, err := s.setupDischarger(p) if err != nil { return nil, errors.E(op, err, "failed to set up discharger") @@ -412,9 +426,57 @@ func (s *Service) setupDischarger(p Params) (*discharger.MacaroonDischarger, err return MacaroonDischarger, nil } -func setupSessionStore(db *sql.DB, secretKey string) (*pgstore.PGStore, error) { - store, err := pgstore.NewPGStoreFromPool(db, []byte(secretKey)) - return store, err +func (s *Service) setupSessionStore(ctx context.Context) (*pgstore.PGStore, error) { + const op = errors.Op("setupSessionStore") + + if s.jimm.CredentialStore == nil { + return nil, errors.E(op, "credential store is not configured") + } + + sqlDb, err := s.jimm.Database.DB.DB() + if err != nil { + return nil, errors.E(op, err) + } + + oauthSessionStoreSecret, err := s.jimm.CredentialStore.GetOAuthSessionStoreSecret(ctx) + if err == nil { + zapctx.Info(ctx, "detected existing OAuth session store secret") + } else if errors.ErrorCode(err) == errors.CodeNotFound { + oauthSessionStoreSecret, err = generateOAuthSessionStoreSecret(ctx, s.jimm.CredentialStore) + if err != nil { + return nil, err + } + } else { + return nil, errors.E(op, err, "failed to read session store secret") + } + + store, err := pgstore.NewPGStoreFromPool(sqlDb, oauthSessionStoreSecret) + if err != nil { + return nil, errors.E(op, err, "failed to create session store") + } + + // Cleanup expired session every 30 minutes + cleanupQuit, cleanupDone := store.Cleanup(time.Minute * 30) + s.AddCleanup(func() error { + store.StopCleanup(cleanupQuit, cleanupDone) + return nil + }) + return store, nil +} + +func generateOAuthSessionStoreSecret(ctx context.Context, store jimmcreds.CredentialStore) ([]byte, error) { + const op = errors.Op("generateOAuthSessionStoreSecret") + secret := make([]byte, 64) + if _, err := rand.Read(secret); err != nil { + zapctx.Error(ctx, "failed to generate OAuth session store secret", zap.Error(err)) + return nil, errors.E(op, err, "failed to generate OAuth session store secret") + } + + if err := store.PutOAuthSessionStoreSecret(ctx, secret); err != nil { + zapctx.Error(ctx, "failed to store generated OAuth session store secret", zap.Error(err)) + return nil, errors.E(op, err, "failed to store generated OAuth session store secret") + } + return secret, nil } func openDB(ctx context.Context, dsn string) (*gorm.DB, error) { @@ -458,9 +520,7 @@ func (s *Service) setupCredentialStore(ctx context.Context, p Params) error { return nil } - // Currently jimm will start without a credential store but - // functionality will be limited. - return nil + return errors.E(op, "jimm cannot start without a credential store") } func newVaultStore(ctx context.Context, p Params) (jimmcreds.CredentialStore, error) { diff --git a/service_test.go b/service_test.go index eba70964a..9aac190ed 100644 --- a/service_test.go +++ b/service_test.go @@ -42,8 +42,10 @@ func TestDefaultService(t *testing.T) { c.Assert(err, qt.IsNil) p := jimmtest.NewTestJimmParams(c) p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams) + p.InsecureSecretStorage = true svc, err := jimm.NewService(context.Background(), p) c.Assert(err, qt.IsNil) + defer svc.Cleanup() rr := httptest.NewRecorder() req, err := http.NewRequest("GET", "/debug/info", nil) c.Assert(err, qt.IsNil) @@ -52,7 +54,7 @@ func TestDefaultService(t *testing.T) { c.Check(resp.StatusCode, qt.Equals, http.StatusOK) } -func TestServiceStartsWithoutSecretStore(t *testing.T) { +func TestServiceDoesNotStartWithoutCredentialStore(t *testing.T) { c := qt.New(t) _, _, cofgaParams, err := jimmtest.SetupTestOFGAClient(c.Name()) @@ -60,7 +62,7 @@ func TestServiceStartsWithoutSecretStore(t *testing.T) { p := jimmtest.NewTestJimmParams(c) p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams) _, err = jimm.NewService(context.Background(), p) - c.Assert(err, qt.IsNil) + c.Assert(err, qt.ErrorMatches, "jimm cannot start without a credential store") } func TestAuthenticator(t *testing.T) { @@ -75,6 +77,7 @@ func TestAuthenticator(t *testing.T) { p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams) svc, err := jimm.NewService(context.Background(), p) c.Assert(err, qt.IsNil) + defer svc.Cleanup() err = svc.JIMM().GetCredentialStore().PutOAuthSecret(ctx, []byte(jimmtest.JWTTestSecret)) c.Assert(err, qt.IsNil) @@ -139,6 +142,7 @@ func TestVault(t *testing.T) { p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams) svc, err := jimm.NewService(ctx, p) c.Assert(err, qt.IsNil) + defer svc.Cleanup() err = svc.JIMM().GetCredentialStore().PutOAuthSecret(ctx, []byte(jimmtest.JWTTestSecret)) c.Assert(err, qt.IsNil) @@ -197,8 +201,9 @@ func TestPostgresSecretStore(t *testing.T) { p := jimmtest.NewTestJimmParams(c) p.InsecureSecretStorage = true p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams) - _, err = jimm.NewService(context.Background(), p) + svc, err := jimm.NewService(context.Background(), p) c.Assert(err, qt.IsNil) + defer svc.Cleanup() } func TestOpenFGA(t *testing.T) { @@ -214,6 +219,7 @@ func TestOpenFGA(t *testing.T) { p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams) svc, err := jimm.NewService(ctx, p) c.Assert(err, qt.IsNil) + defer svc.Cleanup() err = svc.JIMM().GetCredentialStore().PutOAuthSecret(ctx, []byte(jimmtest.JWTTestSecret)) c.Assert(err, qt.IsNil) @@ -260,8 +266,10 @@ func TestPublicKey(t *testing.T) { p := jimmtest.NewTestJimmParams(c) p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams) + p.InsecureSecretStorage = true svc, err := jimm.NewService(context.Background(), p) c.Assert(err, qt.IsNil) + defer svc.Cleanup() srv := httptest.NewTLSServer(svc) c.Cleanup(srv.Close) @@ -334,8 +342,10 @@ func TestThirdPartyCaveatDischarge(t *testing.T) { p := jimmtest.NewTestJimmParams(c) p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams) + p.InsecureSecretStorage = true svc, err := jimm.NewService(context.Background(), p) c.Assert(err, qt.IsNil) + defer svc.Cleanup() srv := httptest.NewTLSServer(svc) c.Cleanup(srv.Close) @@ -392,6 +402,51 @@ func TestThirdPartyCaveatDischarge(t *testing.T) { } } +func TestDisableOAuthEndpointsWhenDashboardRedirectURLNotSet(t *testing.T) { + c := qt.New(t) + + _, _, cofgaParams, err := jimmtest.SetupTestOFGAClient(c.Name()) + c.Assert(err, qt.IsNil) + + p := jimmtest.NewTestJimmParams(c) + p.DashboardFinalRedirectURL = "" + p.InsecureSecretStorage = true + p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams) + svc, err := jimm.NewService(context.Background(), p) + c.Assert(err, qt.IsNil) + defer svc.Cleanup() + + srv := httptest.NewTLSServer(svc) + c.Cleanup(srv.Close) + + response, err := srv.Client().Get(srv.URL + "/auth/whoami") + c.Assert(err, qt.IsNil) + c.Assert(response.StatusCode, qt.Equals, http.StatusNotFound) +} + +func TestEnableOAuthEndpointsWhenDashboardRedirectURLSet(t *testing.T) { + c := qt.New(t) + + _, _, cofgaParams, err := jimmtest.SetupTestOFGAClient(c.Name()) + c.Assert(err, qt.IsNil) + + p := jimmtest.NewTestJimmParams(c) + p.DashboardFinalRedirectURL = "some-redirect-url" + p.InsecureSecretStorage = true + p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams) + + svc, err := jimm.NewService(context.Background(), p) + c.Assert(err, qt.IsNil) + defer svc.Cleanup() + + srv := httptest.NewTLSServer(svc) + c.Cleanup(srv.Close) + + response, err := srv.Client().Get(srv.URL + "/auth/whoami") + c.Assert(err, qt.IsNil) + c.Assert(response.StatusCode, qt.Not(qt.Equals), http.StatusNotFound) +} + // cofgaParamsToJIMMOpenFGAParams To avoid circular references, the test setup function (jimmtest.SetupTestOFGAClient) // does not provide us with an instance of `jimm.OpenFGAParams`, so it just returns a `cofga.OpenFGAParams` instance. // This method reshapes the later into the former. @@ -405,3 +460,32 @@ func cofgaParamsToJIMMOpenFGAParams(cofgaParams cofga.OpenFGAParams) jimm.OpenFG AuthModel: cofgaParams.AuthModelID, } } + +func TestCleanup(t *testing.T) { + c := qt.New(t) + + outputs := make(chan string, 2) + service := jimm.Service{} + service.AddCleanup(func() error { outputs <- "first"; return nil }) + service.AddCleanup(func() error { outputs <- "second"; return nil }) + service.Cleanup() + c.Assert([]string{<-outputs, <-outputs}, qt.DeepEquals, []string{"second", "first"}) +} + +func TestCleanupDoesNotPanic_SessionStoreRelatedCleanups(t *testing.T) { + c := qt.New(t) + + _, _, cofgaParams, err := jimmtest.SetupTestOFGAClient(c.Name()) + c.Assert(err, qt.IsNil) + p := jimmtest.NewTestJimmParams(c) + p.OpenFGAParams = cofgaParamsToJIMMOpenFGAParams(*cofgaParams) + p.InsecureSecretStorage = true + + svc, err := jimm.NewService(context.Background(), p) + c.Assert(err, qt.IsNil) + + // Make sure `cleanups` is not empty. + c.Assert(len(svc.GetCleanups()) > 0, qt.IsTrue) + + svc.Cleanup() +}