From a3654445da9779baf974be4a63bb026776cb5ca3 Mon Sep 17 00:00:00 2001 From: Kian Parvin <46668016+kian99@users.noreply.github.com> Date: Mon, 3 Jun 2024 17:43:37 +0200 Subject: [PATCH] Add TLSHostname field to controller model (#1225) * Add TLSHostname field to controller model * Added tests * Reduce log level on isAdmin check * Tweak error message --- api/params/params.go | 3 ++ cmd/jimmctl/cmd/controllerinfo.go | 9 ++-- cmd/jimmctl/cmd/controllerinfo_test.go | 59 +++++++++++++++++++++++++- internal/dbmodel/controller.go | 4 ++ internal/dbmodel/sql/postgres/1_8.sql | 4 ++ internal/dbmodel/version.go | 2 +- internal/jujuapi/jimm.go | 1 + internal/jujuapi/jimm_test.go | 37 ++++++++++++++++ internal/openfga/user.go | 2 +- internal/rpc/dial.go | 3 +- 10 files changed, 117 insertions(+), 7 deletions(-) create mode 100644 internal/dbmodel/sql/postgres/1_8.sql diff --git a/api/params/params.go b/api/params/params.go index 2666833e5..42e39116d 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"` 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/internal/dbmodel/controller.go b/internal/dbmodel/controller.go index 92297f36c..5704aecd0 100644 --- a/internal/dbmodel/controller.go +++ b/internal/dbmodel/controller.go @@ -46,6 +46,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 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/version.go b/internal/dbmodel/version.go index 2c113e675..dc772ead4 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 = 8 ) type Version struct { 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..ed10b0a1c 100644 --- a/internal/jujuapi/jimm_test.go +++ b/internal/jujuapi/jimm_test.go @@ -186,6 +186,43 @@ func (s *jimmSuite) TestAddController(c *gc.C) { c.Assert(jujuparams.IsBadRequest(err), gc.Equals, true) } +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/openfga/user.go b/internal/openfga/user.go index ea8a4e380..40255f2ab 100644 --- a/internal/openfga/user.go +++ b/internal/openfga/user.go @@ -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()), 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{