From 742e1d8ff020e87325730e8146f1af51e017e0d4 Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Wed, 10 Jul 2024 09:40:29 +0100 Subject: [PATCH 01/28] docs: add conventional commits guidelines to contributing.md Conventional commtis were mentioned in the TLDR of the document but not in the body. The section is added along with a link to the office juju spec on the matter. --- CONTRIBUTING.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2a0dd28582f..edcd74ce114 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -301,6 +301,39 @@ with an environment variable: JUJU_NOTEST_MONGOJS=1 go test github.com/juju/juju/... ``` +Conventional commits +-------------------- + +Once you have written some code and have tested the changes, the next step is to +`git commit` it. For commit messages Juju follows the conventional commits guidelines specified [in this +document](https://docs.google.com/document/d/1SYUo9G7qZ_jdoVXpUVamS5VCgHmtZ0QA-wZxKoMS-C0/). +In short the commits should be of the following form: +``` +(optional ): + +[optional body] + +[optional footer(s)] +``` +- **Type:** The type describes the kind of change (e.g., feat, fix, docs, style, + refactor, test, chore). +- **Scope:** The scope indicates the part of the codebase affected (e.g., model, + api, cli). +- **Description:** The description briefly describes the change. It should not + end in any punctuation. +- **Body:** The body should be a detailed explanation of the change. It should + specify what has been changed and why and include descriptions of the + behaviour before and after. +- **Footer:** The footer includes information about breaking changes, issues + closed, etc. + +The type, scope and description should all begin with a lower case letter. None +of the lines can exceed 100 characters in length. + +There is a CI action on the Juju GitHub repository that will flag any syntactic +issues with your commit messages. The action is required to pass before code can +be merged so make sure to take a look once your PR is up. + Pushing ------- @@ -318,6 +351,9 @@ Go to the web page (https://github.com/$YOUR_GITHUB_USERNAME/juju) and hit the This creates a numbered pull request on the github site, where members of the Juju project can see and comment on the changes. +The title of the PR should match the form of the title of a conventional commit. +This can be the title of the most significant commit in the PR. + Make sure to add a clear description of why and what has been changed, and include the Launchpad bug number if one exists. From ee26e64b2ba2ad21650ebbd5dd1dba8d3b6c65f4 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Tue, 16 Jul 2024 09:48:44 +0100 Subject: [PATCH 02/28] test: verify current profiles Change the verify current profiles to only notice the difference from the expected and provider profiles. As difference is only a left aligned difference, it won't notice if additional items are added to the expected profiles. Checking the length upfront prevents that from being an issue. --- worker/instancemutater/mutater.go | 13 ++++++------- worker/instancemutater/mutater_test.go | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/worker/instancemutater/mutater.go b/worker/instancemutater/mutater.go index 88da8e45b5e..5c953c2befa 100644 --- a/worker/instancemutater/mutater.go +++ b/worker/instancemutater/mutater.go @@ -304,16 +304,15 @@ func (m MutaterMachine) verifyCurrentProfiles(instID string, expectedProfiles [] if err != nil { return false, err } - obtainedSet := set.NewStrings(obtainedProfiles...) - expectedSet := set.NewStrings(expectedProfiles...) - if obtainedSet.Union(expectedSet).Size() > obtainedSet.Size() { + if len(obtainedProfiles) == 0 && len(expectedProfiles) == 0 { + return true, nil + } else if len(obtainedProfiles) != len(expectedProfiles) { return false, nil } - if expectedSet.Union(obtainedSet).Size() > expectedSet.Size() { - return false, nil - } + obtainedSet := set.NewStrings(obtainedProfiles...) + expectedSet := set.NewStrings(expectedProfiles...) - return true, nil + return obtainedSet.Difference(expectedSet).Size() == 0, nil } diff --git a/worker/instancemutater/mutater_test.go b/worker/instancemutater/mutater_test.go index 77474cfbafb..1fc8c16ddb0 100644 --- a/worker/instancemutater/mutater_test.go +++ b/worker/instancemutater/mutater_test.go @@ -204,6 +204,26 @@ func (s *mutaterSuite) TestVerifyCurrentProfilesFalseContents(c *gc.C) { c.Assert(ok, jc.IsFalse) } +func (s *mutaterSuite) TestVerifyCurrentProfilesFalseContentsWithMissingExpectedProfiles(c *gc.C) { + defer s.setUpMocks(c).Finish() + + s.expectLXDProfileNames([]string{"default", "juju-testme", "juju-testme-lxd-profile-0"}, nil) + + ok, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, []string{"default", "juju-testme"}) + c.Assert(err, jc.ErrorIsNil) + c.Assert(ok, jc.IsFalse) +} + +func (s *mutaterSuite) TestVerifyCurrentProfilesFalseContentsWithMissingProviderProfiles(c *gc.C) { + defer s.setUpMocks(c).Finish() + + s.expectLXDProfileNames([]string{"default", "juju-testme"}, nil) + + ok, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, []string{"default", "juju-testme", "juju-testme-lxd-profile-0"}) + c.Assert(err, jc.ErrorIsNil) + c.Assert(ok, jc.IsFalse) +} + func (s *mutaterSuite) TestVerifyCurrentProfilesError(c *gc.C) { defer s.setUpMocks(c).Finish() From cd35aba646aebe542f931cff095f60f92ea52189 Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Tue, 16 Jul 2024 21:10:08 +1000 Subject: [PATCH 03/28] fix: reflect unchanged lxd profiles back to the machine --- worker/instancemutater/export_test.go | 2 +- worker/instancemutater/mutater.go | 16 +++++++-------- worker/instancemutater/mutater_test.go | 27 +++++++++++++++++--------- worker/instancemutater/worker_test.go | 10 +++++++--- 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/worker/instancemutater/export_test.go b/worker/instancemutater/export_test.go index ef5653463d0..a0cbd0d7164 100644 --- a/worker/instancemutater/export_test.go +++ b/worker/instancemutater/export_test.go @@ -63,6 +63,6 @@ func GatherProfileData(m *MutaterMachine, info *instancemutater.UnitProfileInfo) return m.gatherProfileData(info) } -func VerifyCurrentProfiles(m *MutaterMachine, instId string, expectedProfiles []string) (bool, error) { +func VerifyCurrentProfiles(m *MutaterMachine, instId string, expectedProfiles []string) (bool, []string, error) { return m.verifyCurrentProfiles(instId, expectedProfiles) } diff --git a/worker/instancemutater/mutater.go b/worker/instancemutater/mutater.go index 5c953c2befa..8b7c959f173 100644 --- a/worker/instancemutater/mutater.go +++ b/worker/instancemutater/mutater.go @@ -237,13 +237,13 @@ func (m MutaterMachine) processMachineProfileChanges(info *instancemutater.UnitP } } - verified, err := m.verifyCurrentProfiles(string(info.InstanceId), expectedProfiles) + verified, currentProfiles, err := m.verifyCurrentProfiles(string(info.InstanceId), expectedProfiles) if err != nil { return report(errors.Annotatef(err, "%s", m.id)) } if verified { m.logger.Infof("no changes necessary to machine-%s lxd profiles (%v)", m.id, expectedProfiles) - return report(nil) + return report(m.machineApi.SetCharmProfiles(currentProfiles)) } // Adding a wrench to test charm not running hooks before profile can be applied. @@ -259,7 +259,7 @@ func (m MutaterMachine) processMachineProfileChanges(info *instancemutater.UnitP m.logger.Infof("machine-%s (%s) assign lxd profiles %q, %#v", m.id, string(info.InstanceId), expectedProfiles, post) broker := m.context.getBroker() - currentProfiles, err := broker.AssignLXDProfiles(string(info.InstanceId), expectedProfiles, post) + currentProfiles, err = broker.AssignLXDProfiles(string(info.InstanceId), expectedProfiles, post) if err != nil { m.logger.Errorf("failure to assign lxd profiles %s to machine-%s: %s", expectedProfiles, m.id, err) return report(err) @@ -298,21 +298,21 @@ func (m MutaterMachine) gatherProfileData(info *instancemutater.UnitProfileInfo) return result, nil } -func (m MutaterMachine) verifyCurrentProfiles(instID string, expectedProfiles []string) (bool, error) { +func (m MutaterMachine) verifyCurrentProfiles(instID string, expectedProfiles []string) (bool, []string, error) { broker := m.context.getBroker() obtainedProfiles, err := broker.LXDProfileNames(instID) if err != nil { - return false, err + return false, nil, err } if len(obtainedProfiles) == 0 && len(expectedProfiles) == 0 { - return true, nil + return true, obtainedProfiles, nil } else if len(obtainedProfiles) != len(expectedProfiles) { - return false, nil + return false, obtainedProfiles, nil } obtainedSet := set.NewStrings(obtainedProfiles...) expectedSet := set.NewStrings(expectedProfiles...) - return obtainedSet.Difference(expectedSet).Size() == 0, nil + return obtainedSet.Difference(expectedSet).Size() == 0, obtainedProfiles, nil } diff --git a/worker/instancemutater/mutater_test.go b/worker/instancemutater/mutater_test.go index 1fc8c16ddb0..1a476ff5594 100644 --- a/worker/instancemutater/mutater_test.go +++ b/worker/instancemutater/mutater_test.go @@ -178,9 +178,10 @@ func (s *mutaterSuite) TestVerifyCurrentProfilesTrue(c *gc.C) { profiles := []string{"default", "juju-testme", "juju-testme-lxd-profile-0"} s.expectLXDProfileNames(profiles, nil) - ok, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, profiles) + ok, obtained, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, profiles) c.Assert(err, jc.ErrorIsNil) c.Assert(ok, jc.IsTrue) + c.Assert(obtained, jc.DeepEquals, profiles) } func (s *mutaterSuite) TestVerifyCurrentProfilesFalseLength(c *gc.C) { @@ -189,39 +190,46 @@ func (s *mutaterSuite) TestVerifyCurrentProfilesFalseLength(c *gc.C) { profiles := []string{"default", "juju-testme", "juju-testme-lxd-profile-0"} s.expectLXDProfileNames(profiles, nil) - ok, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, append(profiles, "juju-testme-next-1")) + ok, obtained, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, append(profiles, "juju-testme-next-1")) c.Assert(err, jc.ErrorIsNil) c.Assert(ok, jc.IsFalse) + c.Assert(obtained, jc.DeepEquals, profiles) } func (s *mutaterSuite) TestVerifyCurrentProfilesFalseContents(c *gc.C) { defer s.setUpMocks(c).Finish() - s.expectLXDProfileNames([]string{"default", "juju-testme", "juju-testme-lxd-profile-0"}, nil) + profiles := []string{"default", "juju-testme", "juju-testme-lxd-profile-0"} + s.expectLXDProfileNames(profiles, nil) - ok, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, []string{"default", "juju-testme", "juju-testme-lxd-profile-1"}) + ok, obtained, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, []string{"default", "juju-testme", "juju-testme-lxd-profile-1"}) c.Assert(err, jc.ErrorIsNil) c.Assert(ok, jc.IsFalse) + c.Assert(obtained, jc.DeepEquals, profiles) } func (s *mutaterSuite) TestVerifyCurrentProfilesFalseContentsWithMissingExpectedProfiles(c *gc.C) { defer s.setUpMocks(c).Finish() - s.expectLXDProfileNames([]string{"default", "juju-testme", "juju-testme-lxd-profile-0"}, nil) + profiles := []string{"default", "juju-testme", "juju-testme-lxd-profile-0"} + s.expectLXDProfileNames(profiles, nil) - ok, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, []string{"default", "juju-testme"}) + ok, obtained, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, []string{"default", "juju-testme"}) c.Assert(err, jc.ErrorIsNil) c.Assert(ok, jc.IsFalse) + c.Assert(obtained, jc.DeepEquals, profiles) } func (s *mutaterSuite) TestVerifyCurrentProfilesFalseContentsWithMissingProviderProfiles(c *gc.C) { defer s.setUpMocks(c).Finish() - s.expectLXDProfileNames([]string{"default", "juju-testme"}, nil) + profiles := []string{"default", "juju-testme"} + s.expectLXDProfileNames(profiles, nil) - ok, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, []string{"default", "juju-testme", "juju-testme-lxd-profile-0"}) + ok, obtained, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, []string{"default", "juju-testme", "juju-testme-lxd-profile-0"}) c.Assert(err, jc.ErrorIsNil) c.Assert(ok, jc.IsFalse) + c.Assert(obtained, jc.DeepEquals, profiles) } func (s *mutaterSuite) TestVerifyCurrentProfilesError(c *gc.C) { @@ -229,9 +237,10 @@ func (s *mutaterSuite) TestVerifyCurrentProfilesError(c *gc.C) { s.expectLXDProfileNames([]string{}, errors.NotFoundf("instId")) - ok, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, []string{"default"}) + ok, obtained, err := instancemutater.VerifyCurrentProfiles(s.mutaterMachine, s.instId, []string{"default"}) c.Assert(err, jc.Satisfies, errors.IsNotFound) c.Assert(ok, jc.IsFalse) + c.Assert(obtained, gc.IsNil) } func (s *mutaterSuite) setUpMocks(c *gc.C) *gomock.Controller { diff --git a/worker/instancemutater/worker_test.go b/worker/instancemutater/worker_test.go index 59f5f10afa6..98efcacc576 100644 --- a/worker/instancemutater/worker_test.go +++ b/worker/instancemutater/worker_test.go @@ -4,6 +4,7 @@ package instancemutater_test import ( + "fmt" "strconv" "sync" "time" @@ -202,7 +203,7 @@ func (s *workerEnvironSuite) TestFullWorkflow(c *gc.C) { s.notifyMachineAppLXDProfile(0, 1) s.expectMachineCharmProfilingInfo(0, 3) s.expectLXDProfileNamesTrue() - s.expectSetCharmProfiles(0) + s.expectSetCharmProfiles(0, 3) s.expectAssignLXDProfiles() s.expectAliveAndSetModificationStatusIdle(0) s.expectModificationStatusApplied(0) @@ -219,6 +220,7 @@ func (s *workerEnvironSuite) TestVerifyCurrentProfilesTrue(c *gc.C) { s.expectAliveAndSetModificationStatusIdle(0) s.expectMachineCharmProfilingInfo(0, 2) s.expectLXDProfileNamesTrue() + s.expectSetCharmProfiles(0, 2) s.expectModificationStatusApplied(0) s.cleanKill(c, s.workerForScenario(c)) @@ -256,6 +258,8 @@ func (s *workerEnvironSuite) TestMachineNotifyTwice(c *gc.C) { s.expectMachineCharmProfilingInfo(1, 2) s.expectLXDProfileNamesTrue() s.expectLXDProfileNamesTrue() + s.expectSetCharmProfiles(0, 2) + s.expectSetCharmProfiles(1, 2) s.expectMachineAliveStatusIdleMachineDead(0, &group) s.cleanKill(c, s.workerForScenario(c)) @@ -516,8 +520,8 @@ func (s *workerSuite) expectAssignLXDProfiles() { s.broker.EXPECT().AssignLXDProfiles("juju-23423-0", profiles, gomock.Any()).Return(profiles, nil) } -func (s *workerSuite) expectSetCharmProfiles(machine int) { - s.machine[machine].EXPECT().SetCharmProfiles([]string{"default", "juju-testing", "juju-testing-one-3"}) +func (s *workerSuite) expectSetCharmProfiles(machine int, rev int) { + s.machine[machine].EXPECT().SetCharmProfiles([]string{"default", "juju-testing", fmt.Sprintf("juju-testing-one-%d", rev)}) } func (s *workerSuite) expectRemoveAllCharmProfiles(machine int) { From c5cd45859b83f944a48926662a46ac09c49078b0 Mon Sep 17 00:00:00 2001 From: Nicolas Vinuesa Date: Wed, 17 Jul 2024 17:30:07 +0200 Subject: [PATCH 04/28] fix(machine): clean up controller units when removing controller machines This patch fixes an issue that appears when trying to remove a controller machine from an HA cluster. The fix is basically to reintroduce the cleanup that was used before https://github.com/juju/juju/pull/17653. --- state/cleanup.go | 59 +++++++++++++++++++++++++++++++++++++ state/cleanup_test.go | 66 +++++++++++++++++++++++++++++++++++------- state/enableha_test.go | 5 ++-- state/machine.go | 40 +++++++++++-------------- state/machine_test.go | 4 ++- 5 files changed, 137 insertions(+), 37 deletions(-) diff --git a/state/cleanup.go b/state/cleanup.go index 6a3459844a9..a3f48614d93 100644 --- a/state/cleanup.go +++ b/state/cleanup.go @@ -46,6 +46,7 @@ const ( cleanupDyingMachine cleanupKind = "dyingMachine" cleanupForceDestroyedMachine cleanupKind = "machine" cleanupForceRemoveMachine cleanupKind = "forceRemoveMachine" + cleanupEvacuateMachine cleanupKind = "evacuateMachine" cleanupAttachmentsForDyingStorage cleanupKind = "storageAttachments" cleanupAttachmentsForDyingVolume cleanupKind = "volumeAttachments" cleanupAttachmentsForDyingFilesystem cleanupKind = "filesystemAttachments" @@ -221,6 +222,8 @@ func (st *State) Cleanup() (err error) { err = st.cleanupForceRemoveMachine(doc.Prefix, args) case cleanupAttachmentsForDyingStorage: err = st.cleanupAttachmentsForDyingStorage(doc.Prefix, args) + case cleanupEvacuateMachine: + err = st.cleanupEvacuateMachine(doc.Prefix, args) case cleanupAttachmentsForDyingVolume: err = st.cleanupAttachmentsForDyingVolume(doc.Prefix) case cleanupAttachmentsForDyingFilesystem: @@ -1436,6 +1439,62 @@ func (st *State) cleanupForceRemoveMachine(machineId string, cleanupArgs []bson. return machine.Remove() } +// cleanupEvacuateMachine is initiated by machine.Destroy() to gracefully remove units +// from the machine before then kicking off machine destroy. +func (st *State) cleanupEvacuateMachine(machineId string, cleanupArgs []bson.Raw) error { + if len(cleanupArgs) > 0 { + return errors.Errorf("expected no arguments, got %d", len(cleanupArgs)) + } + + machine, err := st.Machine(machineId) + if errors.IsNotFound(err) { + return nil + } else if err != nil { + return errors.Trace(err) + } + if machine.Life() != Alive { + return nil + } + + units, err := machine.Units() + if err != nil { + return errors.Trace(err) + } + + if len(units) == 0 { + if err := machine.advanceLifecycle(Dying, false, false, 0); err != nil { + return errors.Trace(err) + } + return nil + } + + buildTxn := func(attempt int) ([]txn.Op, error) { + if attempt > 0 { + units, err = machine.Units() + if err != nil { + return nil, errors.Trace(err) + } + } + var ops []txn.Op + for _, unit := range units { + destroyOp := unit.DestroyOperation() + op, err := destroyOp.Build(attempt) + if err != nil && !errors.Is(err, jujutxn.ErrNoOperations) { + return nil, errors.Trace(err) + } + ops = append(ops, op...) + } + return ops, nil + } + + err = st.db().Run(buildTxn) + if err != nil { + return errors.Trace(err) + } + + return errors.Errorf("waiting for units to be removed from %s", machineId) +} + // cleanupContainers recursively calls cleanupForceDestroyedMachine on the supplied // machine's containers, and removes them from state entirely. func (st *State) cleanupContainers(machine *Machine, maxWait time.Duration) error { diff --git a/state/cleanup_test.go b/state/cleanup_test.go index f2c085a679e..9a3094358a0 100644 --- a/state/cleanup_test.go +++ b/state/cleanup_test.go @@ -6,6 +6,7 @@ package state_test import ( "bytes" "sort" + "strconv" "time" "github.com/juju/charm/v12" @@ -473,6 +474,56 @@ func (s *CleanupSuite) TestDestroyControllerMachineErrors(c *gc.C) { assertLife(c, manager, state.Alive) } +func (s *CleanupSuite) TestDestroyControllerMachineHAWithControllerCharm(c *gc.C) { + cons := constraints.Value{ + Mem: newUint64(100), + } + changes, err := s.State.EnableHA(3, cons, state.UbuntuBase("12.04"), nil) + c.Assert(err, jc.ErrorIsNil) + c.Assert(changes.Added, gc.HasLen, 3) + + ch := s.AddTestingCharmWithSeries(c, "juju-controller", "") + app := s.AddTestingApplicationForBase(c, state.UbuntuBase("12.04"), "controller", ch) + + var machines []*state.Machine + var units []*state.Unit + for i := 0; i < 3; i++ { + m, err := s.State.Machine(strconv.Itoa(i)) + c.Assert(err, jc.ErrorIsNil) + c.Assert(m.Jobs(), gc.DeepEquals, []state.MachineJob{ + state.JobHostUnits, + state.JobManageModel, + }) + + if i == 0 { + node, err := s.State.ControllerNode(m.Id()) + c.Assert(err, jc.ErrorIsNil) + node.SetHasVote(true) + } + + u, err := app.AddUnit(state.AddUnitParams{}) + c.Assert(err, jc.ErrorIsNil) + err = u.SetCharmURL(ch.URL()) + c.Assert(err, jc.ErrorIsNil) + err = u.AssignToMachine(m) + c.Assert(err, jc.ErrorIsNil) + + machines = append(machines, m) + units = append(units, u) + } + + for _, m := range machines { + assertLife(c, m, state.Alive) + } + for _, u := range units { + assertLife(c, u, state.Alive) + } + + s.assertDoesNotNeedCleanup(c) + err = machines[2].Destroy() + c.Assert(err, gc.ErrorMatches, ".* has unit .* assigned") +} + const dontWait = time.Duration(0) func (s *CleanupSuite) TestCleanupForceDestroyedMachineUnit(c *gc.C) { @@ -534,7 +585,6 @@ func (s *CleanupSuite) TestCleanupForceDestroyedControllerMachine(c *gc.C) { // The machine should no longer want the vote, should be forced to not have the vote, and forced to not be a // controller member anymore c.Assert(machine.Refresh(), jc.ErrorIsNil) - c.Check(machine.Life(), gc.Equals, state.Dying) node, err = s.State.ControllerNode(machine.Id()) c.Assert(err, jc.ErrorIsNil) c.Check(node.WantsVote(), jc.IsFalse) @@ -548,21 +598,15 @@ func (s *CleanupSuite) TestCleanupForceDestroyedControllerMachine(c *gc.C) { s.assertCleanupRuns(c) c.Assert(node.SetHasVote(false), jc.ErrorIsNil) // However, if we remove the vote, it can be cleaned up. - // ForceDestroy sets up a cleanupForceDestroyedMachine, which - // calls advanceLifecycle(Dead) which sets up a - // cleanupDyingMachine, which in turn creates a delayed - // cleanupForceRemoveMachine. - // Run the first two. - s.assertCleanupCountDirty(c, 2) - // After we've run the cleanup for the controller machine, the machine should be dead, and it should not be + // ForceDestroy sets up a cleanupEvacuateMachine, which will not + // add any other cleanup ops. + // After we've run the cleanup for the controller machine, the machine should be dying, and it should not be // present in the other documents. - assertLife(c, machine, state.Dead) + assertLife(c, machine, state.Dying) controllerIds, err = s.State.ControllerIds() c.Assert(err, jc.ErrorIsNil) sort.Strings(controllerIds) sort.Strings(changes.Added) - // Only the machines that were added should still be part of the controller - c.Check(controllerIds, gc.DeepEquals, changes.Added) } func (s *CleanupSuite) TestCleanupForceDestroyMachineCleansStorageAttachments(c *gc.C) { diff --git a/state/enableha_test.go b/state/enableha_test.go index 49ba19bcb1d..1413f6c4dd2 100644 --- a/state/enableha_test.go +++ b/state/enableha_test.go @@ -525,8 +525,9 @@ func (s *EnableHASuite) TestForceDestroyFromHA(c *gc.C) { err = m0.ForceDestroy(dontWait) c.Assert(err, jc.ErrorIsNil) c.Assert(m0.Refresh(), jc.ErrorIsNil) - // Could this actually get all the way to Dead? - c.Check(m0.Life(), gc.Equals, state.Dying) + // Force remove of controller machines will first clean up units and + // after that it will pass the machine life to Dying. + c.Check(m0.Life(), gc.Equals, state.Alive) c.Assert(node.Refresh(), jc.ErrorIsNil) c.Assert(node.WantsVote(), jc.IsFalse) } diff --git a/state/machine.go b/state/machine.go index a9e2d367009..7b0910181d8 100644 --- a/state/machine.go +++ b/state/machine.go @@ -639,31 +639,25 @@ func (m *Machine) forceDestroyOps(maxWait time.Duration) ([]txn.Op, error) { if len(controllerIds) <= 1 { return nil, errors.Errorf("controller %s is the only controller", m.Id()) } - // We set the machine to Dying if it isn't already dead. - var machineOp txn.Op - if m.Life() < Dead { - // Make sure we don't want the vote, and we are queued to be Dying. - // Since we are force deleting, life assert should be current machine's life. - machineOp = txn.Op{ - C: machinesC, - Id: m.doc.DocID, - Assert: bson.D{{"life", bson.D{{"$in", []Life{Alive, Dying}}}}}, - Update: bson.D{{"$set", bson.D{{"life", Dying}}}}, - } - } - controllerOp := txn.Op{ - C: controllersC, - Id: modelGlobalKey, - Assert: bson.D{{"controller-ids", controllerIds}}, - } - // Note that ForceDestroy does *not* cleanup the replicaset, so it might cause problems. - // However, we're letting the user handle times when the machine agent isn't running, etc. - // We may need to update the peergrouper for this. return []txn.Op{ - machineOp, - controllerOp, + { + C: machinesC, + Id: m.doc.DocID, + Assert: bson.D{ + {"life", Alive}, + advanceLifecycleUnitAsserts(m.doc.Principals), + }, + // To prevent race conditions, we remove the ability for new + // units to be deployed to the machine. + Update: bson.D{{"$pull", bson.D{{"jobs", JobHostUnits}}}}, + }, + { + C: controllersC, + Id: modelGlobalKey, + Assert: bson.D{{"controller-ids", controllerIds}}, + }, setControllerWantsVoteOp(m.st, m.Id(), false), - newCleanupOp(cleanupForceDestroyedMachine, m.doc.Id, maxWait), + newCleanupOp(cleanupEvacuateMachine, m.doc.Id), }, nil } else { // Make sure the machine doesn't become a manager while we're destroying it diff --git a/state/machine_test.go b/state/machine_test.go index 2a544a8ad04..e19ffdb2c4b 100644 --- a/state/machine_test.go +++ b/state/machine_test.go @@ -374,7 +374,9 @@ func (s *MachineSuite) TestLifeJobManageModelWithControllerCharm(c *gc.C) { err = m2.Refresh() c.Assert(err, jc.ErrorIsNil) - c.Assert(m2.Life(), gc.Equals, state.Dying) + // Force remove of controller machines will first clean up units and + // after that it will pass the machine life to Dying. + c.Assert(m2.Life(), gc.Equals, state.Alive) cn2, err := s.State.ControllerNode(m2.Id()) c.Assert(err, jc.ErrorIsNil) From 6941f27a1e8a27c2423ecde5eeaa09c20d3f19a8 Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Thu, 18 Jul 2024 17:04:25 +0100 Subject: [PATCH 05/28] docs: add conventional commits spec in markdown format This is a copy of the private to canonical doc found here: https://docs.google.com/document/d/1SYUo9G7qZ_jdoVXpUVamS5VCgHmtZ0QA-wZxKoMS-C0/edit --- CONTRIBUTING.md | 3 +- doc/conventional-commits.md | 153 ++++++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 doc/conventional-commits.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index edcd74ce114..9e872595e79 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -305,8 +305,7 @@ Conventional commits -------------------- Once you have written some code and have tested the changes, the next step is to -`git commit` it. For commit messages Juju follows the conventional commits guidelines specified [in this -document](https://docs.google.com/document/d/1SYUo9G7qZ_jdoVXpUVamS5VCgHmtZ0QA-wZxKoMS-C0/). +`git commit` it. For commit messages Juju follows [conventional commits guidelines](doc/conventional-commits.md). In short the commits should be of the following form: ``` (optional ): diff --git a/doc/conventional-commits.md b/doc/conventional-commits.md new file mode 100644 index 00000000000..3bc01b2ac1b --- /dev/null +++ b/doc/conventional-commits.md @@ -0,0 +1,153 @@ +The [Conventional Commits standard](https://www.conventionalcommits.org/en/v1.0.0/) is adopted for the Juju project with the +following structure: + +``` +(optional ): + +[optional body] + +[optional footer(s)] +``` + +- **type**: Describes the kind of change (e.g., feat, fix, docs, style, refactor, test, chore). + +|Type|Kind of change|Description| +|---|---|---| +|feat|Features|A new feature| +|fix|Bug fixes|A bug fix| +|docs|Documentation|Documentation only changes| +|style|Styles|Changes that do not affect the meaning of the code| +|refactor|Code refactoring|A code change that neither fixes a bug nor adds a feature| +|perf|Performance improvements|A code change that improves performance| +|test|Tests|Adding missing tests or correcting existing tests| +|build|Builds|Changes that affect the build system or external dependencies| +|ci|Continuous integration|Changes to our CI configuration files and scripts| +|chore|Chores|Necessary yet mundane/trivial changes (updating dependencies, merging through …)| +|revert|Reverts|Reverts a previous commit| + +- **scope**: A scope indicating the part of the codebase affected (e.g., model, api, cli). + - Can be Optional is it’s hard to define the scope +- **description**: A brief summary of the change. + - Must be provided after the BREAKING CHANGE:, describing what has changed about the API, e.g., BREAKING CHANGE: environment variables now take precedence over config files. + - Should use lowercase. + - Should not end in any punctuation. +- **body**: Detailed explanation of the change. + - Can be Optional for small/trivial `fix`, but NOT for other types. + - Could consist of several paragraphs. + - Explanation of change should details what was before, and what is after, and avoid contextual terms like 'now' + - Good body should be on the form: + * Before this commit `` + * After this commit `` +- **footer**: (Optional) Information about breaking changes, issues closed, etc. + - A commit can contain a footer which must consist of one or more [Git trailers](https://git-scm.com/docs/git-interpret-trailers). + - The footer must start at the first occurrence of a blank line, followed by a Git trailer. + - Each trailer must start on its own line, using the format ``. + - The trailer `` must be either BREAKING CHANGE or be one or more words, grouped by hyphens (e.g. Co-Authored-By, fixes, etc.). + - The trailer `` must be one of `:` or `#`, supporting both `Co-Authored-By: Thomas Miller ` and `Fixes #999`. + - The trailer `` must be present and can contain any characters, either on a single line, split over multiple lines, or split over multiple paragraphs. + + +Here is the reference example of a conventional commit message: + +``` +feat(api): add user authentication feature + +# scope: Indicates the part of the codebase affected +# Here, 'api' is specified as the scope + +# description: A brief summary of the change in lower-case +# "add user authentication feature" summarizes the change concisely + +This commit adds user authentication to the API. Users can now sign up, +log in, and log out. Passwords are hashed using bcrypt. Token-based  +authentication is implemented using JWT. + +# body: Detailed explanation of the change +# A more detailed explanation of what has been changed and why + +BREAKING CHANGE: The user authentication changes the login endpoint +from `/api/login` to `/api/v1/login`. All previous tokens are now invalid, +and users will need to reauthenticate. + +# footer: Information about breaking changes +# Here, the footer indicates a breaking change, describing what has changed +# about the API and how it impacts users + +Authored-By: Thomas Miller <[thomas.miller@canonical.com](mailto:thomas.miller@canonical.com)> + +Fixes #123 + +# footer: Issues closed or other information +# This footer references to and author and an issue that this commit closes. +``` + +The PR title, where a conventional commit is introduced, should be the same format as a first line of a commit description: `(optional ): `. In case this commit is core/base/most significant for this PR. + +### Footer and Multi-paragraph Body +### Juju-Specific Scopes + +Define specific scopes relevant to the Juju project to enhance clarity. Examples include: + +- model: Logical grouping of applications, representing an environment. +- controller: Manages and oversees multiple models, providing centralized management. +- charm: Encapsulated operational code for applications, defining how they are deployed and managed. +- application: Deployed instance of a charm, representing a running service. +- unit: Individual instance of an application, running on a machine. +- relation: Connection between different applications, enabling them to communicate and share data. +- bundle: Collection of applications and relations, defining a complex deployment setup. +- action: Scripted operation on an application or unit, allowing for specific tasks to be performed. +- machine: Physical or virtual host running units, part of the infrastructure. +- storage: Persistent data storage for units, ensuring data remains available across restarts. +- offer: Exposed application endpoint to other models, allowing cross-model relations. +- endpoint: Specific interaction point for a relation within an application. +- constraint: Resource specifications for machines and applications, such as CPU and memory requirements. +- user: Individual with access to the Juju environment, assigned roles and permissions. +- space: Logical network segmentation within models, aiding in network isolation and security. +- subnet: IP address range within a space, used for assigning IPs to machines and units. +- backup: Process of saving the state of a model or controller for recovery purposes. +- migration: Moving a model or application from one controller to another. +- resource: Files or data required by charms during deployment and operation. +- upgrade: Process of updating charms, applications, or Juju itself to newer versions. +- cloud: Infrastructure provider where models and applications are deployed, such as AWS, Azure, or OpenStack. +- credential: Authentication details for accessing cloud providers and resources. + +This list just illustrates the idea and suggestion of which single words should be used for scope definition. It is NOT complete and is NOT limited by these scopes. + +### Other Examples +``` +feat(networking): add juju networking support for all provider + + +``` +--- +``` +feat!: remove ticket list endpoint + +refers to JIRA-666 + +BREAKING CHANGES: ticket enpoints no longer supports list all entites. +``` +--- +``` +fix(migration): fix the universal migration to any major/minor version in future + + +``` +--- +``` +docs: add support for multi-version + tips windows + + +``` +--- +``` +refactor(cmr): remove all uclear and understandle code from CMR implementation + + +``` +--- +``` +fix(backup): fix backup db state issue + + +``` \ No newline at end of file From 19901e9b345456361247f7a195045b5ac3db34c4 Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Fri, 19 Jul 2024 09:33:05 +1000 Subject: [PATCH 06/28] fix: only charm profiles saved to instance data --- core/lxdprofile/name.go | 10 +++++----- core/lxdprofile/name_test.go | 2 +- state/machine.go | 18 +++++++++++------- state/machine_test.go | 26 ++++++++++++++++++++++++++ worker/instancemutater/mutater.go | 4 ++-- worker/instancemutater/mutater_test.go | 3 ++- worker/instancemutater/worker_test.go | 6 +++--- worker/provisioner/provisioner_task.go | 2 +- 8 files changed, 51 insertions(+), 20 deletions(-) diff --git a/core/lxdprofile/name.go b/core/lxdprofile/name.go index 930b5322245..4783709eb24 100644 --- a/core/lxdprofile/name.go +++ b/core/lxdprofile/name.go @@ -18,7 +18,7 @@ const AppName = "juju" // Prefix is used to prefix all the lxd profile programmable profiles. If a // profile doesn't have the prefix, then it will be removed when ensuring the -// the validity of the names (see LXDProfileNames) +// the validity of the names (see FilterLXDProfileNames) var Prefix = fmt.Sprintf("%s-", AppName) // Name returns a serialisable name that we can use to identify profiles @@ -27,10 +27,10 @@ func Name(modelName, appName string, revision int) string { return fmt.Sprintf("%s%s-%s-%d", Prefix, modelName, appName, revision) } -// LXDProfileNames ensures that the LXD profile names are unique yet preserve +// FilterLXDProfileNames ensures that the LXD profile names are unique yet preserve // the same order as the input. It removes certain profile names from the list, // for example "default" profile name will be removed. -func LXDProfileNames(names []string) []string { +func FilterLXDProfileNames(names []string) []string { // ensure that the ones we have are unique unique := make(map[string]int) for k, v := range names { @@ -111,10 +111,10 @@ func MatchProfileNameByAppName(names []string, appName string) (string, error) { return "", errors.BadRequestf("no application name specified") } var foundProfile string - for _, p := range LXDProfileNames(names) { + for _, p := range FilterLXDProfileNames(names) { rev, err := ProfileRevision(p) if err != nil { - // "Shouldn't" happen since we used LXDProfileNames... + // "Shouldn't" happen since we used FilterLXDProfileNames... if errors.IsBadRequest(err) { continue } diff --git a/core/lxdprofile/name_test.go b/core/lxdprofile/name_test.go index 86cfcb8a9fc..c4519b1b41f 100644 --- a/core/lxdprofile/name_test.go +++ b/core/lxdprofile/name_test.go @@ -73,7 +73,7 @@ func (*LXDProfileNameSuite) TestProfileNames(c *gc.C) { } for k, tc := range testCases { c.Logf("running test %d with input %q", k, tc.input) - c.Assert(lxdprofile.LXDProfileNames(tc.input), gc.DeepEquals, tc.output) + c.Assert(lxdprofile.FilterLXDProfileNames(tc.input), gc.DeepEquals, tc.output) } } diff --git a/state/machine.go b/state/machine.go index a9e2d367009..b6ac368c45e 100644 --- a/state/machine.go +++ b/state/machine.go @@ -430,29 +430,33 @@ func (m *Machine) CharmProfiles() ([]string, error) { // SetCharmProfiles sets the names of the charm profiles used on a machine // in its instanceData. func (m *Machine) SetCharmProfiles(profiles []string) error { - if len(profiles) == 0 { - return nil - } buildTxn := func(attempt int) ([]txn.Op, error) { if attempt > 0 { if err := m.Refresh(); err != nil { return nil, errors.Trace(err) } } + // Exit early if the Machine profiles doesn't need to change. - mProfiles, err := m.CharmProfiles() + currentProfiles, err := m.CharmProfiles() if err != nil { return nil, errors.Trace(err) } - mProfilesSet := set.NewStrings(mProfiles...) - if mProfilesSet.Union(set.NewStrings(profiles...)).Size() == mProfilesSet.Size() { + currentProfileSet := set.NewStrings(currentProfiles...) + newProfileSet := set.NewStrings(profiles...) + if currentProfileSet.Size() == newProfileSet.Size() && + currentProfileSet.Intersection(newProfileSet).Size() == currentProfileSet.Size() { return nil, jujutxn.ErrNoOperations } + assertion := bson.M{"charm-profiles": currentProfiles} + if currentProfiles == nil { + assertion = bson.M{"charm-profiles": bson.D{{"$exists", false}}} + } ops := []txn.Op{{ C: instanceDataC, Id: m.doc.DocID, - Assert: txn.DocExists, + Assert: assertion, Update: bson.D{{"$set", bson.D{{"charm-profiles", profiles}}}}, }} diff --git a/state/machine_test.go b/state/machine_test.go index 2a544a8ad04..be2f6e1b645 100644 --- a/state/machine_test.go +++ b/state/machine_test.go @@ -890,6 +890,32 @@ func (s *MachineSuite) TestMachineCharmProfiles(c *gc.C) { c.Assert(err, jc.ErrorIsNil) saved = all.CharmProfiles(s.machine.Id()) c.Assert(saved, jc.SameContents, profiles) + + // set to nil + err = s.machine.SetCharmProfiles(nil) + c.Assert(err, jc.ErrorIsNil) + + saved, err = s.machine.CharmProfiles() + c.Assert(err, jc.ErrorIsNil) + c.Assert(saved, gc.HasLen, 0) + + all, err = s.Model.AllInstanceData() + c.Assert(err, jc.ErrorIsNil) + saved = all.CharmProfiles(s.machine.Id()) + c.Assert(saved, gc.HasLen, 0) + + // set back to non-nil + err = s.machine.SetCharmProfiles(profiles) + c.Assert(err, jc.ErrorIsNil) + + saved, err = s.machine.CharmProfiles() + c.Assert(err, jc.ErrorIsNil) + c.Assert(saved, jc.SameContents, profiles) + + all, err = s.Model.AllInstanceData() + c.Assert(err, jc.ErrorIsNil) + saved = all.CharmProfiles(s.machine.Id()) + c.Assert(saved, jc.SameContents, profiles) } func (s *MachineSuite) TestMachineAvailabilityZone(c *gc.C) { diff --git a/worker/instancemutater/mutater.go b/worker/instancemutater/mutater.go index 8b7c959f173..d971fa11f20 100644 --- a/worker/instancemutater/mutater.go +++ b/worker/instancemutater/mutater.go @@ -243,7 +243,7 @@ func (m MutaterMachine) processMachineProfileChanges(info *instancemutater.UnitP } if verified { m.logger.Infof("no changes necessary to machine-%s lxd profiles (%v)", m.id, expectedProfiles) - return report(m.machineApi.SetCharmProfiles(currentProfiles)) + return report(m.machineApi.SetCharmProfiles(lxdprofile.FilterLXDProfileNames(currentProfiles))) } // Adding a wrench to test charm not running hooks before profile can be applied. @@ -265,7 +265,7 @@ func (m MutaterMachine) processMachineProfileChanges(info *instancemutater.UnitP return report(err) } - return report(m.machineApi.SetCharmProfiles(currentProfiles)) + return report(m.machineApi.SetCharmProfiles(lxdprofile.FilterLXDProfileNames(currentProfiles))) } func (m MutaterMachine) gatherProfileData(info *instancemutater.UnitProfileInfo) ([]lxdprofile.ProfilePost, error) { diff --git a/worker/instancemutater/mutater_test.go b/worker/instancemutater/mutater_test.go index 1a476ff5594..bfe7e057797 100644 --- a/worker/instancemutater/mutater_test.go +++ b/worker/instancemutater/mutater_test.go @@ -47,11 +47,12 @@ func (s *mutaterSuite) TestProcessMachineProfileChanges(c *gc.C) { startingProfiles := []string{"default", "juju-testme"} finishingProfiles := append(startingProfiles, "juju-testme-lxd-profile-1") + charmProfiles := []string{"juju-testme-lxd-profile-1"} s.expectRefreshLifeAliveStatusIdle() s.expectLXDProfileNames(startingProfiles, nil) s.expectAssignLXDProfiles(finishingProfiles, nil) - s.expectSetCharmProfiles(finishingProfiles) + s.expectSetCharmProfiles(charmProfiles) s.expectModificationStatusApplied() info := s.info(startingProfiles, 1, true) diff --git a/worker/instancemutater/worker_test.go b/worker/instancemutater/worker_test.go index 98efcacc576..a5a23474c64 100644 --- a/worker/instancemutater/worker_test.go +++ b/worker/instancemutater/worker_test.go @@ -521,12 +521,12 @@ func (s *workerSuite) expectAssignLXDProfiles() { } func (s *workerSuite) expectSetCharmProfiles(machine int, rev int) { - s.machine[machine].EXPECT().SetCharmProfiles([]string{"default", "juju-testing", fmt.Sprintf("juju-testing-one-%d", rev)}) + s.machine[machine].EXPECT().SetCharmProfiles([]string{fmt.Sprintf("juju-testing-one-%d", rev)}) } func (s *workerSuite) expectRemoveAllCharmProfiles(machine int) { profiles := []string{"default", "juju-testing"} - s.machine[machine].EXPECT().SetCharmProfiles(profiles) + s.machine[machine].EXPECT().SetCharmProfiles([]string{}) s.broker.EXPECT().AssignLXDProfiles("juju-23423-0", profiles, gomock.Any()).Return(profiles, nil) } @@ -741,7 +741,7 @@ func (s *workerContainerSuite) expectAssignLXDProfiles() { } func (s *workerContainerSuite) expectContainerSetCharmProfiles() { - s.lxdContainer.EXPECT().SetCharmProfiles([]string{"default", "juju-testing-one-3"}) + s.lxdContainer.EXPECT().SetCharmProfiles([]string{"juju-testing-one-3"}) } // notifyContainers returns a suite behaviour that will cause the instance mutator diff --git a/worker/provisioner/provisioner_task.go b/worker/provisioner/provisioner_task.go index eae1a1c96b3..96b3e7f7360 100644 --- a/worker/provisioner/provisioner_task.go +++ b/worker/provisioner/provisioner_task.go @@ -1583,7 +1583,7 @@ func (task *provisionerTask) gatherCharmLXDProfiles( return nil, errors.Trace(err) } - return lxdprofile.LXDProfileNames(profileNames), nil + return lxdprofile.FilterLXDProfileNames(profileNames), nil } // markMachineFailedInAZ moves the machine in zone from MachineIds to FailedMachineIds From 371b56d261190f841cd4fe1780ed1a1d4f25c812 Mon Sep 17 00:00:00 2001 From: Alastair Flynn Date: Fri, 19 Jul 2024 12:19:18 +0100 Subject: [PATCH 07/28] fix: auth for external users Auth for external user was broken because if the user is not local, we check their username against the logged in user name from state, and the local username we were checking it against was empty. This was because of a check against local users that would set their account detials to empty if they had no macaroons. The macaroons are not stored in the accountdetails of the external user in this case, and so the username it was compared against was empty. This fix sets the account details to the username and macaroons (which may be empty). --- cmd/modelcmd/base.go | 16 +- cmd/modelcmd/base_test.go | 39 ++++ cmd/modelcmd/mocks/api_mock.go | 391 +++++++++++++++++++++++++++++++++ cmd/modelcmd/package_test.go | 1 + 4 files changed, 437 insertions(+), 10 deletions(-) create mode 100644 cmd/modelcmd/mocks/api_mock.go diff --git a/cmd/modelcmd/base.go b/cmd/modelcmd/base.go index 7c0d3d4e5c9..6fb3fec192a 100644 --- a/cmd/modelcmd/base.go +++ b/cmd/modelcmd/base.go @@ -235,16 +235,12 @@ func (c *CommandBase) NewAPIRootWithDialOpts( } else { u := names.NewUserTag(accountDetails.User) if !u.IsLocal() { - if len(accountDetails.Macaroons) == 0 { - accountDetails = &jujuclient.AccountDetails{} - } else { - // If the account has macaroon set, use those to login - // to avoid an unnecessary auth round trip. - // Used for embedded commands. - accountDetails = &jujuclient.AccountDetails{ - User: u.Id(), - Macaroons: accountDetails.Macaroons, - } + // If the account has macaroon set, use those to login + // to avoid an unnecessary auth round trip. + // Used for embedded commands. + accountDetails = &jujuclient.AccountDetails{ + User: u.Id(), + Macaroons: accountDetails.Macaroons, } } } diff --git a/cmd/modelcmd/base_test.go b/cmd/modelcmd/base_test.go index 9fa34444a45..bdc96b408e7 100644 --- a/cmd/modelcmd/base_test.go +++ b/cmd/modelcmd/base_test.go @@ -13,9 +13,11 @@ import ( "github.com/juju/cmd/v3" "github.com/juju/cmd/v3/cmdtesting" "github.com/juju/errors" + "github.com/juju/names/v5" cookiejar "github.com/juju/persistent-cookiejar" "github.com/juju/testing" jc "github.com/juju/testing/checkers" + "go.uber.org/mock/gomock" gc "gopkg.in/check.v1" "gopkg.in/macaroon-bakery.v2/httpbakery" "gopkg.in/macaroon.v2" @@ -24,6 +26,7 @@ import ( apitesting "github.com/juju/juju/api/testing" "github.com/juju/juju/cloud" "github.com/juju/juju/cmd/modelcmd" + "github.com/juju/juju/cmd/modelcmd/mocks" "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" "github.com/juju/juju/environs" @@ -189,6 +192,42 @@ To access it run 'juju switch bar:admin/badmodel'.`, } } +func (s *BaseCommandSuite) setupMocks(c *gc.C) *gomock.Controller { + ctrl := gomock.NewController(c) + return ctrl +} + +func (s *BaseCommandSuite) TestNewAPIRootExternalUser(c *gc.C) { + ctrl := s.setupMocks(c) + conn := mocks.NewMockConnection(ctrl) + apiOpen := func(info *api.Info, opts api.DialOpts) (api.Connection, error) { + return conn, nil + } + externalName := "alastair@external" + conn.EXPECT().AuthTag().Return(names.NewUserTag(externalName)).MinTimes(1) + conn.EXPECT().APIHostPorts() + conn.EXPECT().ServerVersion() + conn.EXPECT().Addr() + conn.EXPECT().IPAddr() + conn.EXPECT().PublicDNSName() + conn.EXPECT().ControllerAccess().MinTimes(1) + + s.store.Accounts["foo"] = jujuclient.AccountDetails{ + User: externalName, + } + + baseCmd := new(modelcmd.ModelCommandBase) + baseCmd.SetClientStore(s.store) + baseCmd.SetAPIOpen(apiOpen) + modelcmd.InitContexts(&cmd.Context{Stderr: io.Discard}, baseCmd) + modelcmd.SetRunStarted(baseCmd) + + c.Assert(baseCmd.SetModelIdentifier("foo:admin/badmodel", false), jc.ErrorIsNil) + + _, err := baseCmd.NewAPIRoot() + c.Assert(err, jc.ErrorIsNil) +} + type NewGetBootstrapConfigParamsFuncSuite struct { testing.IsolationSuite } diff --git a/cmd/modelcmd/mocks/api_mock.go b/cmd/modelcmd/mocks/api_mock.go new file mode 100644 index 00000000000..4d8fe92c87c --- /dev/null +++ b/cmd/modelcmd/mocks/api_mock.go @@ -0,0 +1,391 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/juju/juju/api (interfaces: Connection) +// +// Generated by this command: +// +// mockgen -package mocks -destination mocks/api_mock.go github.com/juju/juju/api Connection +// + +// Package mocks is a generated GoMock package. +package mocks + +import ( + context "context" + http "net/http" + url "net/url" + reflect "reflect" + + base "github.com/juju/juju/api/base" + network "github.com/juju/juju/core/network" + proxy "github.com/juju/juju/proxy" + names "github.com/juju/names/v5" + version "github.com/juju/version/v2" + gomock "go.uber.org/mock/gomock" + httprequest "gopkg.in/httprequest.v1" + macaroon "gopkg.in/macaroon.v2" +) + +// MockConnection is a mock of Connection interface. +type MockConnection struct { + ctrl *gomock.Controller + recorder *MockConnectionMockRecorder +} + +// MockConnectionMockRecorder is the mock recorder for MockConnection. +type MockConnectionMockRecorder struct { + mock *MockConnection +} + +// NewMockConnection creates a new mock instance. +func NewMockConnection(ctrl *gomock.Controller) *MockConnection { + mock := &MockConnection{ctrl: ctrl} + mock.recorder = &MockConnectionMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockConnection) EXPECT() *MockConnectionMockRecorder { + return m.recorder +} + +// APICall mocks base method. +func (m *MockConnection) APICall(arg0 string, arg1 int, arg2, arg3 string, arg4, arg5 any) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "APICall", arg0, arg1, arg2, arg3, arg4, arg5) + ret0, _ := ret[0].(error) + return ret0 +} + +// APICall indicates an expected call of APICall. +func (mr *MockConnectionMockRecorder) APICall(arg0, arg1, arg2, arg3, arg4, arg5 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "APICall", reflect.TypeOf((*MockConnection)(nil).APICall), arg0, arg1, arg2, arg3, arg4, arg5) +} + +// APIHostPorts mocks base method. +func (m *MockConnection) APIHostPorts() []network.MachineHostPorts { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "APIHostPorts") + ret0, _ := ret[0].([]network.MachineHostPorts) + return ret0 +} + +// APIHostPorts indicates an expected call of APIHostPorts. +func (mr *MockConnectionMockRecorder) APIHostPorts() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "APIHostPorts", reflect.TypeOf((*MockConnection)(nil).APIHostPorts)) +} + +// Addr mocks base method. +func (m *MockConnection) Addr() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Addr") + ret0, _ := ret[0].(string) + return ret0 +} + +// Addr indicates an expected call of Addr. +func (mr *MockConnectionMockRecorder) Addr() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Addr", reflect.TypeOf((*MockConnection)(nil).Addr)) +} + +// AuthTag mocks base method. +func (m *MockConnection) AuthTag() names.Tag { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AuthTag") + ret0, _ := ret[0].(names.Tag) + return ret0 +} + +// AuthTag indicates an expected call of AuthTag. +func (mr *MockConnectionMockRecorder) AuthTag() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AuthTag", reflect.TypeOf((*MockConnection)(nil).AuthTag)) +} + +// BakeryClient mocks base method. +func (m *MockConnection) BakeryClient() base.MacaroonDischarger { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BakeryClient") + ret0, _ := ret[0].(base.MacaroonDischarger) + return ret0 +} + +// BakeryClient indicates an expected call of BakeryClient. +func (mr *MockConnectionMockRecorder) BakeryClient() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BakeryClient", reflect.TypeOf((*MockConnection)(nil).BakeryClient)) +} + +// BestFacadeVersion mocks base method. +func (m *MockConnection) BestFacadeVersion(arg0 string) int { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BestFacadeVersion", arg0) + ret0, _ := ret[0].(int) + return ret0 +} + +// BestFacadeVersion indicates an expected call of BestFacadeVersion. +func (mr *MockConnectionMockRecorder) BestFacadeVersion(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BestFacadeVersion", reflect.TypeOf((*MockConnection)(nil).BestFacadeVersion), arg0) +} + +// Broken mocks base method. +func (m *MockConnection) Broken() <-chan struct{} { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Broken") + ret0, _ := ret[0].(<-chan struct{}) + return ret0 +} + +// Broken indicates an expected call of Broken. +func (mr *MockConnectionMockRecorder) Broken() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Broken", reflect.TypeOf((*MockConnection)(nil).Broken)) +} + +// Close mocks base method. +func (m *MockConnection) Close() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Close") + ret0, _ := ret[0].(error) + return ret0 +} + +// Close indicates an expected call of Close. +func (mr *MockConnectionMockRecorder) Close() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockConnection)(nil).Close)) +} + +// ConnectControllerStream mocks base method. +func (m *MockConnection) ConnectControllerStream(arg0 string, arg1 url.Values, arg2 http.Header) (base.Stream, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ConnectControllerStream", arg0, arg1, arg2) + ret0, _ := ret[0].(base.Stream) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ConnectControllerStream indicates an expected call of ConnectControllerStream. +func (mr *MockConnectionMockRecorder) ConnectControllerStream(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConnectControllerStream", reflect.TypeOf((*MockConnection)(nil).ConnectControllerStream), arg0, arg1, arg2) +} + +// ConnectStream mocks base method. +func (m *MockConnection) ConnectStream(arg0 string, arg1 url.Values) (base.Stream, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ConnectStream", arg0, arg1) + ret0, _ := ret[0].(base.Stream) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ConnectStream indicates an expected call of ConnectStream. +func (mr *MockConnectionMockRecorder) ConnectStream(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConnectStream", reflect.TypeOf((*MockConnection)(nil).ConnectStream), arg0, arg1) +} + +// Context mocks base method. +func (m *MockConnection) Context() context.Context { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Context") + ret0, _ := ret[0].(context.Context) + return ret0 +} + +// Context indicates an expected call of Context. +func (mr *MockConnectionMockRecorder) Context() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Context", reflect.TypeOf((*MockConnection)(nil).Context)) +} + +// ControllerAccess mocks base method. +func (m *MockConnection) ControllerAccess() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ControllerAccess") + ret0, _ := ret[0].(string) + return ret0 +} + +// ControllerAccess indicates an expected call of ControllerAccess. +func (mr *MockConnectionMockRecorder) ControllerAccess() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControllerAccess", reflect.TypeOf((*MockConnection)(nil).ControllerAccess)) +} + +// ControllerTag mocks base method. +func (m *MockConnection) ControllerTag() names.ControllerTag { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ControllerTag") + ret0, _ := ret[0].(names.ControllerTag) + return ret0 +} + +// ControllerTag indicates an expected call of ControllerTag. +func (mr *MockConnectionMockRecorder) ControllerTag() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControllerTag", reflect.TypeOf((*MockConnection)(nil).ControllerTag)) +} + +// CookieURL mocks base method. +func (m *MockConnection) CookieURL() *url.URL { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CookieURL") + ret0, _ := ret[0].(*url.URL) + return ret0 +} + +// CookieURL indicates an expected call of CookieURL. +func (mr *MockConnectionMockRecorder) CookieURL() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CookieURL", reflect.TypeOf((*MockConnection)(nil).CookieURL)) +} + +// HTTPClient mocks base method. +func (m *MockConnection) HTTPClient() (*httprequest.Client, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "HTTPClient") + ret0, _ := ret[0].(*httprequest.Client) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// HTTPClient indicates an expected call of HTTPClient. +func (mr *MockConnectionMockRecorder) HTTPClient() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HTTPClient", reflect.TypeOf((*MockConnection)(nil).HTTPClient)) +} + +// IPAddr mocks base method. +func (m *MockConnection) IPAddr() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IPAddr") + ret0, _ := ret[0].(string) + return ret0 +} + +// IPAddr indicates an expected call of IPAddr. +func (mr *MockConnectionMockRecorder) IPAddr() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IPAddr", reflect.TypeOf((*MockConnection)(nil).IPAddr)) +} + +// IsBroken mocks base method. +func (m *MockConnection) IsBroken() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsBroken") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsBroken indicates an expected call of IsBroken. +func (mr *MockConnectionMockRecorder) IsBroken() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsBroken", reflect.TypeOf((*MockConnection)(nil).IsBroken)) +} + +// IsProxied mocks base method. +func (m *MockConnection) IsProxied() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsProxied") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsProxied indicates an expected call of IsProxied. +func (mr *MockConnectionMockRecorder) IsProxied() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsProxied", reflect.TypeOf((*MockConnection)(nil).IsProxied)) +} + +// Login mocks base method. +func (m *MockConnection) Login(arg0 names.Tag, arg1, arg2 string, arg3 []macaroon.Slice) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Login", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(error) + return ret0 +} + +// Login indicates an expected call of Login. +func (mr *MockConnectionMockRecorder) Login(arg0, arg1, arg2, arg3 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Login", reflect.TypeOf((*MockConnection)(nil).Login), arg0, arg1, arg2, arg3) +} + +// ModelTag mocks base method. +func (m *MockConnection) ModelTag() (names.ModelTag, bool) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ModelTag") + ret0, _ := ret[0].(names.ModelTag) + ret1, _ := ret[1].(bool) + return ret0, ret1 +} + +// ModelTag indicates an expected call of ModelTag. +func (mr *MockConnectionMockRecorder) ModelTag() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ModelTag", reflect.TypeOf((*MockConnection)(nil).ModelTag)) +} + +// Proxy mocks base method. +func (m *MockConnection) Proxy() proxy.Proxier { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Proxy") + ret0, _ := ret[0].(proxy.Proxier) + return ret0 +} + +// Proxy indicates an expected call of Proxy. +func (mr *MockConnectionMockRecorder) Proxy() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Proxy", reflect.TypeOf((*MockConnection)(nil).Proxy)) +} + +// PublicDNSName mocks base method. +func (m *MockConnection) PublicDNSName() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PublicDNSName") + ret0, _ := ret[0].(string) + return ret0 +} + +// PublicDNSName indicates an expected call of PublicDNSName. +func (mr *MockConnectionMockRecorder) PublicDNSName() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PublicDNSName", reflect.TypeOf((*MockConnection)(nil).PublicDNSName)) +} + +// RootHTTPClient mocks base method. +func (m *MockConnection) RootHTTPClient() (*httprequest.Client, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RootHTTPClient") + ret0, _ := ret[0].(*httprequest.Client) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// RootHTTPClient indicates an expected call of RootHTTPClient. +func (mr *MockConnectionMockRecorder) RootHTTPClient() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RootHTTPClient", reflect.TypeOf((*MockConnection)(nil).RootHTTPClient)) +} + +// ServerVersion mocks base method. +func (m *MockConnection) ServerVersion() (version.Number, bool) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ServerVersion") + ret0, _ := ret[0].(version.Number) + ret1, _ := ret[1].(bool) + return ret0, ret1 +} + +// ServerVersion indicates an expected call of ServerVersion. +func (mr *MockConnectionMockRecorder) ServerVersion() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ServerVersion", reflect.TypeOf((*MockConnection)(nil).ServerVersion)) +} diff --git a/cmd/modelcmd/package_test.go b/cmd/modelcmd/package_test.go index 536f95b83b2..8c7b377ab9e 100644 --- a/cmd/modelcmd/package_test.go +++ b/cmd/modelcmd/package_test.go @@ -10,6 +10,7 @@ import ( ) //go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/modelconfig_mock.go github.com/juju/juju/cmd/modelcmd ModelConfigAPI +//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/api_mock.go github.com/juju/juju/api Connection func Test(t *testing.T) { coretesting.MgoTestPackage(t) From 20947d00a6fa8314f637d53a9a506a93577e3980 Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Mon, 22 Jul 2024 21:10:48 +1000 Subject: [PATCH 08/28] fix(migration): empty map compared to nil map for credential equality --- state/cloudcredentials.go | 23 ++++++++---- state/cloudcredentials_test.go | 11 ++++-- state/migration_import.go | 9 ++++- state/migration_import_test.go | 65 ++++++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 10 deletions(-) diff --git a/state/cloudcredentials.go b/state/cloudcredentials.go index dd5fd5e9b3a..a61726c36de 100644 --- a/state/cloudcredentials.go +++ b/state/cloudcredentials.go @@ -308,17 +308,26 @@ func createCloudCredentialOp(tag names.CloudCredentialTag, cred cloud.Credential // updateCloudCredentialOp returns a txn.Op that will update // a cloud credential. func updateCloudCredentialOp(tag names.CloudCredentialTag, cred cloud.Credential) txn.Op { + sets := bson.D{ + {"auth-type", string(cred.AuthType())}, + {"revoked", cred.Revoked}, + {"invalid", cred.Invalid}, + {"invalid-reason", cred.InvalidReason}, + } + attr := cred.Attributes() + if len(attr) == 0 { + // If the attributes are empty, set it to nil. This preserves + // the behaviour of create due to omitempty on the attributes + // field. + sets = append(sets, bson.DocElem{"attributes", nil}) + } else { + sets = append(sets, bson.DocElem{"attributes", attr}) + } return txn.Op{ C: cloudCredentialsC, Id: cloudCredentialDocID(tag), Assert: txn.DocExists, - Update: bson.D{{"$set", bson.D{ - {"auth-type", string(cred.AuthType())}, - {"attributes", cred.Attributes()}, - {"revoked", cred.Revoked}, - {"invalid", cred.Invalid}, - {"invalid-reason", cred.InvalidReason}, - }}}, + Update: bson.D{{"$set", sets}}, } } diff --git a/state/cloudcredentials_test.go b/state/cloudcredentials_test.go index 3e6c3a97db5..847ea4e462a 100644 --- a/state/cloudcredentials_test.go +++ b/state/cloudcredentials_test.go @@ -79,7 +79,7 @@ func (s *CloudCredentialsSuite) TestUpdateCloudCredentialsExisting(c *gc.C) { err := s.State.AddCloud(cloud.Cloud{ Name: "stratus", Type: "low", - AuthTypes: cloud.AuthTypes{cloud.AccessKeyAuthType, cloud.UserPassAuthType}, + AuthTypes: cloud.AuthTypes{cloud.AccessKeyAuthType, cloud.UserPassAuthType, cloud.EmptyAuthType}, }, s.Owner.Name()) c.Assert(err, jc.ErrorIsNil) @@ -111,8 +111,15 @@ func (s *CloudCredentialsSuite) TestUpdateCloudCredentialsExisting(c *gc.C) { expected.Cloud = "stratus" expected.Name = "foobar" expected.Revoked = true - c.Assert(out, jc.DeepEquals, expected) + + // Pass an empty map to check it is returned as a nil map. + cred = cloud.NewCredential(cloud.EmptyAuthType, map[string]string{}) + err = s.State.UpdateCloudCredential(tag, cred) + c.Assert(err, jc.ErrorIsNil) + out, err = s.State.CloudCredential(tag) + c.Assert(err, jc.ErrorIsNil) + c.Assert(out.Attributes, gc.IsNil) } func assertCredentialCreated(c *gc.C, testSuite ConnSuite) (string, *state.User, names.CloudCredentialTag) { diff --git a/state/migration_import.go b/state/migration_import.go index 0081f5100d2..e16ab149a71 100644 --- a/state/migration_import.go +++ b/state/migration_import.go @@ -113,7 +113,7 @@ func (ctrl *Controller) Import(model description.Model) (_ *Model, _ *State, err if existingCreds.AuthType != creds.AuthType() { return nil, nil, errors.Errorf("credential auth type mismatch: %q != %q", existingCreds.AuthType, creds.AuthType()) } - if !reflect.DeepEqual(existingCreds.Attributes, creds.Attributes()) { + if !credentialAttributesMatch(existingCreds.Attributes, creds.Attributes()) { return nil, nil, errors.Errorf("credential attribute mismatch: %v != %v", existingCreds.Attributes, creds.Attributes()) } if existingCreds.Revoked { @@ -253,6 +253,13 @@ func (ctrl *Controller) Import(model description.Model) (_ *Model, _ *State, err return dbModel, newSt, nil } +func credentialAttributesMatch(a map[string]string, b map[string]string) bool { + if len(a) == 0 && len(b) == 0 { + return true + } + return reflect.DeepEqual(a, b) +} + // modelConfig creates a config for the model being imported. func modelConfig(attrs map[string]interface{}) (*config.Config, error) { // If the tools version is before 2.9.35, the default-series diff --git a/state/migration_import_test.go b/state/migration_import_test.go index 5a8fbade2c3..6eccfbcf207 100644 --- a/state/migration_import_test.go +++ b/state/migration_import_test.go @@ -22,6 +22,7 @@ import ( "gopkg.in/juju/environschema.v1" "gopkg.in/yaml.v2" + "github.com/juju/juju/cloud" "github.com/juju/juju/core/arch" corecharm "github.com/juju/juju/core/charm" "github.com/juju/juju/core/constraints" @@ -279,6 +280,70 @@ func (s *MigrationImportSuite) TestModelUsers(c *gc.C) { c.Assert(allUsers, gc.HasLen, 3) } +// TestEmptyCredential checks that when the model uses an empty credential +// that it doesn't matter if the attributes on the source or target controller's +// credential is empty/not-nil, they are both equal. This tests that the controller +// can handle an old or a manually edited credential when it has an empty attribute map +// and is compared to the source controller's nil attribute map (or vice versa). +func (s *MigrationImportSuite) TestEmptyCredential(c *gc.C) { + credTag := names.NewCloudCredentialTag(s.Model.CloudName() + "/" + s.Model.Owner().Id() + "/empty") + cred := cloud.NewEmptyCredential() + err := s.State.UpdateCloudCredential(credTag, cred) + c.Assert(err, jc.ErrorIsNil) + ok, err := s.Model.SetCloudCredential(credTag) + c.Assert(err, jc.ErrorIsNil) + c.Assert(ok, jc.IsTrue) + newModel, _ := s.importModel(c, s.State, func(m map[string]interface{}) { + // Check the the exported credential is omitted. + c.Assert(m["cloud-credential"].(map[any]any)["attributes"], gc.IsNil) + // Force the credential to a non-nil empty map to test nil-map empty-map + // credential check. + m["cloud-credential"].(map[any]any)["attributes"] = map[string]string{} + }) + newCredTag, ok := newModel.CloudCredentialTag() + c.Assert(ok, jc.IsTrue) + c.Assert(newCredTag.Name(), gc.Equals, "empty") +} + +// TestCredentialAttributeMatching checks that the credentials for the target controller +// and the model being imported compare the correct credential attributes. +func (s *MigrationImportSuite) TestCredentialAttributeMatching(c *gc.C) { + // Create foo credential for the "target" controller. + err := s.State.UpdateCloudCredential( + names.NewCloudCredentialTag(s.Model.CloudName()+"/"+s.Model.Owner().Id()+"/bar"), + cloud.NewCredential(cloud.EmptyAuthType, map[string]string{ + "foo": "d09f00d", + })) + c.Assert(err, jc.ErrorIsNil) + + // Create bar credential for the "source" controller + credTag := names.NewCloudCredentialTag(s.Model.CloudName() + "/" + s.Model.Owner().Id() + "/foo") + cred := cloud.NewCredential(cloud.EmptyAuthType, map[string]string{ + "foo": "bar", + }) + err = s.State.UpdateCloudCredential(credTag, cred) + c.Assert(err, jc.ErrorIsNil) + ok, err := s.Model.SetCloudCredential(credTag) + c.Assert(err, jc.ErrorIsNil) + c.Assert(ok, jc.IsTrue) + + newModel, _ := s.importModel(c, s.State, func(m map[string]interface{}) { + // Swap out for the bar credential. + cred := m["cloud-credential"].(map[any]any) + c.Assert(cred["name"], gc.Equals, "foo") + c.Assert(cred["attributes"], gc.DeepEquals, map[any]any{ + "foo": "bar", + }) + cred["name"] = "bar" + cred["attributes"] = map[any]any{ + "foo": "d09f00d", + } + }) + newCredTag, ok := newModel.CloudCredentialTag() + c.Assert(ok, jc.IsTrue) + c.Assert(newCredTag.Name(), gc.Equals, "bar") +} + func (s *MigrationImportSuite) TestSLA(c *gc.C) { err := s.State.SetSLA("essential", "bob", []byte("creds")) c.Assert(err, jc.ErrorIsNil) From 198f7fb019323f3c79d055b9e71d6d4bd101ed0f Mon Sep 17 00:00:00 2001 From: Joseph Phillips Date: Mon, 22 Jul 2024 12:40:33 +0200 Subject: [PATCH 09/28] fix: do not log hook context ID Fixes a potential exploit where a user can run a bash loop attempting to execute hook tools. If running while another hook is executing, we log an error with the context ID, making it possible for the user then use that ID in a following call successfully. --- worker/uniter/runner/runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/worker/uniter/runner/runner.go b/worker/uniter/runner/runner.go index 609b2c22763..5ce52d389ab 100644 --- a/worker/uniter/runner/runner.go +++ b/worker/uniter/runner/runner.go @@ -719,7 +719,7 @@ func (runner *runner) startJujucServer(token string, rMode runMode) (*jujuc.Serv // Prepare server. getCmd := func(ctxId, cmdName string) (cmd.Command, error) { if ctxId != runner.context.Id() { - return nil, errors.Errorf("expected context id %q, got %q", runner.context.Id(), ctxId) + return nil, errors.Errorf("wrong context ID; got %q", ctxId) } return jujuc.NewCommand(runner.context, cmdName) } From d28082dabf05ff855584dbbe3e63ac4313db7135 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Mon, 22 Jul 2024 13:44:38 +0100 Subject: [PATCH 10/28] test: unit test bad context id In order to prove that we've got a bad context ID error, we have to contrive a scenario, so that when the planets align the right way, we get the correct error message. This involves forcing the run action to wait when it's being processed as a remote exection. Then we can call the Jujuc.Main directly using the socket, which then allows us to execute a bad request. A token generator was added to allow us to bypass the token comparison for remote executions. Once all of this is done, we then collapse the server and allow the action to complete. --- I believe this is the first time someone has taken the time to execute this code path outside of integration tests. We should execute this path in unit tests more, either by making the runner more composable, or implementing a better test harness. --- worker/uniter/runner/runner.go | 82 +++++++++++++++++++++------ worker/uniter/runner/runner_test.go | 87 +++++++++++++++++++++++++++++ worker/uniter/util_test.go | 2 +- 3 files changed, 154 insertions(+), 17 deletions(-) diff --git a/worker/uniter/runner/runner.go b/worker/uniter/runner/runner.go index 5ce52d389ab..cb626d5ebf9 100644 --- a/worker/uniter/runner/runner.go +++ b/worker/uniter/runner/runner.go @@ -108,11 +108,41 @@ type Runner interface { } // NewRunnerFunc returns a func used to create a Runner backed by the supplied context and paths. -type NewRunnerFunc func(context context.Context, paths context.Paths, remoteExecutor ExecFunc) Runner +type NewRunnerFunc func(context context.Context, paths context.Paths, remoteExecutor ExecFunc, options ...Option) Runner + +// Option is a functional option for NewRunner. +type Option func(*options) + +type options struct { + tokenGenerator TokenGenerator +} + +// WithTokenGenerator returns an Option that sets the token generator for the +// runner. +func WithTokenGenerator(tg TokenGenerator) Option { + return func(o *options) { + o.tokenGenerator = tg + } +} + +func newOptions() *options { + return &options{ + tokenGenerator: &tokenGenerator{}, + } +} // NewRunner returns a Runner backed by the supplied context and paths. -func NewRunner(context context.Context, paths context.Paths, remoteExecutor ExecFunc) Runner { - return &runner{context, paths, remoteExecutor} +func NewRunner(context context.Context, paths context.Paths, remoteExecutor ExecFunc, options ...Option) Runner { + opts := newOptions() + for _, option := range options { + option(opts) + } + return &runner{ + context: context, + paths: paths, + remoteExecutor: remoteExecutor, + tokenGenerator: opts.tokenGenerator, + } } // ExecParams holds all the necessary parameters for ExecFunc. @@ -152,12 +182,21 @@ func execOnMachine(params ExecParams) (*utilexec.ExecResponse, error) { // ExecFunc is the exec func type. type ExecFunc func(ExecParams) (*utilexec.ExecResponse, error) +// TokenGenerator is the interface for generating tokens. +type TokenGenerator interface { + // Generate generates a token based on the remote flag. + // If remote is false, it returns an empty string. Otherwise, it returns a + // random token. + Generate(remote bool) (string, error) +} + // runner implements Runner. type runner struct { context context.Context paths context.Paths // remoteExecutor executes commands on a remote workload pod for CAAS. remoteExecutor ExecFunc + tokenGenerator TokenGenerator } func (runner *runner) logger() loggo.Logger { @@ -207,14 +246,11 @@ func (runner *runner) RunCommands(commands string, runLocation RunLocation) (*ut // runCommandsWithTimeout is a helper to abstract common code between run commands and // juju-run as an action func (runner *runner) runCommandsWithTimeout(commands string, timeout time.Duration, clock clock.Clock, rMode runMode, abort <-chan struct{}) (*utilexec.ExecResponse, error) { - var err error - token := "" - if rMode == runOnRemote { - token, err = utils.RandomPassword() - if err != nil { - return nil, errors.Trace(err) - } + token, err := runner.tokenGenerator.Generate(rMode == runOnRemote) + if err != nil { + return nil, errors.Trace(err) } + srv, err := runner.startJujucServer(token, rMode) if err != nil { return nil, err @@ -387,13 +423,11 @@ func (runner *runner) RunHook(hookName string) (HookHandlerType, error) { } func (runner *runner) runCharmHookWithLocation(hookName, charmLocation string, rMode runMode) (hookHandlerType HookHandlerType, err error) { - token := "" - if rMode == runOnRemote { - token, err = utils.RandomPassword() - if err != nil { - return InvalidHookHandler, errors.Trace(err) - } + token, err := runner.tokenGenerator.Generate(rMode == runOnRemote) + if err != nil { + return InvalidHookHandler, errors.Trace(err) } + srv, err := runner.startJujucServer(token, rMode) if err != nil { return InvalidHookHandler, errors.Trace(err) @@ -789,3 +823,19 @@ type hookProcess struct { func (p hookProcess) Pid() int { return p.Process.Pid } + +type tokenGenerator struct{} + +// Generate generates a token based on the remote flag. +// If remote is false, it returns an empty string. Otherwise, it returns a +// random token. +func (t *tokenGenerator) Generate(remote bool) (string, error) { + if !remote { + return "", nil + } + token, err := utils.RandomPassword() + if err != nil { + return "", errors.Trace(err) + } + return token, nil +} diff --git a/worker/uniter/runner/runner_test.go b/worker/uniter/runner/runner_test.go index 336ea9bd9b8..e4ac6c6735f 100644 --- a/worker/uniter/runner/runner_test.go +++ b/worker/uniter/runner/runner_test.go @@ -21,10 +21,13 @@ import ( gc "gopkg.in/check.v1" "github.com/juju/juju/core/model" + "github.com/juju/juju/juju/sockets" + "github.com/juju/juju/testing" "github.com/juju/juju/worker/common/charmrunner" "github.com/juju/juju/worker/uniter/hook" "github.com/juju/juju/worker/uniter/runner" "github.com/juju/juju/worker/uniter/runner/context" + "github.com/juju/juju/worker/uniter/runner/jujuc" runnertesting "github.com/juju/juju/worker/uniter/runner/testing" ) @@ -227,6 +230,7 @@ func (s *RunHookSuite) TestRunHookDispatchingHookHandler(c *gc.C) { type MockContext struct { context.Context + id string actionData *context.ActionData actionDataErr error actionParams map[string]interface{} @@ -239,6 +243,10 @@ type MockContext struct { modelType model.ModelType } +func (ctx *MockContext) Id() string { + return ctx.id +} + func (ctx *MockContext) GetLogger(module string) loggo.Logger { return loggo.GetLogger(module) } @@ -322,6 +330,85 @@ func (s *RunMockContextSuite) assertRecordedPid(c *gc.C, expectPid int) { c.Assert(strings.TrimRight(string(content), "\r\n"), gc.Equals, expectContent) } +func (s *RunMockContextSuite) TestBadContextId(c *gc.C) { + params := map[string]interface{}{ + "command": "echo 1", + "timeout": 0, + "workload-context": true, + } + ctx := &MockContext{ + id: "foo-context", + modelType: model.CAAS, + actionData: &context.ActionData{ + Params: params, + }, + actionParams: params, + actionResults: map[string]interface{}{}, + } + start := make(chan struct{}) + done := make(chan struct{}) + result := make(chan error) + execCount := 0 + execFunc := func(params runner.ExecParams) (*exec.ExecResponse, error) { + execCount++ + switch execCount { + case 1: + return &exec.ExecResponse{}, nil + case 2: + close(start) + + select { + case <-done: + case <-time.After(testing.LongWait): + c.Fatalf("timed out waiting to complete") + } + return &exec.ExecResponse{}, nil + } + return nil, nil + } + go func() { + defer close(done) + select { + case <-start: + case <-time.After(testing.LongWait): + c.Fatalf("timed out waiting to start") + } + socket := s.paths.GetJujucServerSocket(false) + + client, err := sockets.Dial(socket) + c.Assert(err, jc.ErrorIsNil) + defer client.Close() + + req := jujuc.Request{ + ContextId: "whatever", + Dir: c.MkDir(), + CommandName: "remote", + } + + var resp exec.ExecResponse + err = client.Call("Jujuc.Main", req, &resp) + + go func() { + result <- err + }() + }() + _, err := runner.NewRunner(ctx, s.paths, execFunc, runner.WithTokenGenerator(localTokenGenerator{})).RunAction("juju-run") + c.Assert(err, jc.ErrorIsNil) + + select { + case err := <-result: + c.Assert(err, gc.ErrorMatches, `.*wrong context ID; got "whatever"`) + case <-time.After(5 * time.Second): + c.Fatal("timed out waiting for jujuc to finish") + } +} + +type localTokenGenerator struct{} + +func (localTokenGenerator) Generate(remote bool) (string, error) { + return "", nil +} + func (s *RunMockContextSuite) TestRunHookFlushSuccess(c *gc.C) { expectErr := errors.New("pew pew pew") ctx := &MockContext{ diff --git a/worker/uniter/util_test.go b/worker/uniter/util_test.go index faacdc22a32..7de5272c2ea 100644 --- a/worker/uniter/util_test.go +++ b/worker/uniter/util_test.go @@ -551,7 +551,7 @@ func (s startUniter) step(c *gc.C, ctx *testContext) { MachineLock: processLock, UpdateStatusSignal: ctx.updateStatusHookTicker.ReturnTimer(), NewOperationExecutor: operationExecutor, - NewProcessRunner: func(context runnercontext.Context, paths runnercontext.Paths, remoteExecutor runner.ExecFunc) runner.Runner { + NewProcessRunner: func(context runnercontext.Context, paths runnercontext.Paths, remoteExecutor runner.ExecFunc, options ...runner.Option) runner.Runner { ctx.runner.ctx = context return ctx.runner }, From 5e81efe153b294e73687f956c2b36c92df8726c0 Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Tue, 23 Jul 2024 10:09:23 +1000 Subject: [PATCH 11/28] test: fix TestSeriesTools to use a supported series even on noble --- apiserver/common/tools_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apiserver/common/tools_test.go b/apiserver/common/tools_test.go index 20f7e61e883..4dc1d23a7ef 100644 --- a/apiserver/common/tools_test.go +++ b/apiserver/common/tools_test.go @@ -129,7 +129,7 @@ func (s *getToolsSuite) TestSeriesTools(c *gc.C) { current := coretesting.CurrentVersion() currentCopy := current - currentCopy.Release = coretesting.HostSeries(c) + currentCopy.Release = "jammy" configAttrs := map[string]interface{}{ "name": "some-name", "type": "some-type", From 90de1918378f76e20cc846105828493318ba6932 Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Tue, 23 Jul 2024 10:48:54 +1000 Subject: [PATCH 12/28] ci: remove installed lxd --- .github/workflows/upgrade.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/upgrade.yml b/.github/workflows/upgrade.yml index f17643511d0..212157c180e 100644 --- a/.github/workflows/upgrade.yml +++ b/.github/workflows/upgrade.yml @@ -45,6 +45,12 @@ jobs: if: env.RUN_TEST == 'RUN' uses: actions/checkout@v3 + - name: Remove LXD + if: env.RUN_TEST == 'RUN' + run: | + set -euxo pipefail + sudo snap remove lxd --purge + - name: Setup LXD if: env.RUN_TEST == 'RUN' && matrix.model_type == 'localhost' uses: canonical/setup-lxd@90d76101915da56a42a562ba766b1a77019242fd From 4cd9d61cf3d786e6631fcc770785637414235226 Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Tue, 23 Jul 2024 13:54:29 +1000 Subject: [PATCH 13/28] chore(deps): security updates Vulnerability #1: GO-2024-2958 Vulnerability #2: GO-2024-2918 Vulnerability #3: GO-2024-2611 --- go.mod | 22 +++++++++++----------- go.sum | 49 +++++++++++++++++++++++++------------------------ 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/go.mod b/go.mod index d30c27653c9..a842dc93a4f 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module github.com/juju/juju go 1.21 require ( - github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.0 - github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.4.0 + github.com/Azure/azure-sdk-for-go/sdk/azcore v1.12.0 + github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.6.0 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v3 v3.0.0-beta.2 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v2 v2.0.0 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/keyvault/armkeyvault v1.4.0 @@ -36,9 +36,9 @@ require ( github.com/gofrs/uuid v4.2.0+incompatible github.com/google/gnostic-models v0.6.8 github.com/google/go-querystring v1.1.0 - github.com/google/uuid v1.4.0 + github.com/google/uuid v1.6.0 github.com/gorilla/handlers v1.3.0 - github.com/gorilla/schema v1.2.0 + github.com/gorilla/schema v1.4.1 github.com/gorilla/websocket v1.5.0 github.com/gosuri/uitable v0.0.1 github.com/hashicorp/go-hclog v0.9.1 @@ -133,10 +133,10 @@ require ( require ( cloud.google.com/go/compute/metadata v0.3.0 // indirect - github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.0 // indirect + github.com/Azure/azure-sdk-for-go/sdk/internal v1.9.0 // indirect github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.0.0 // indirect github.com/Azure/go-ntlmssp v0.0.0-20211209120228-48547f28849e // indirect - github.com/AzureAD/microsoft-authentication-library-for-go v1.1.1 // indirect + github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 // indirect github.com/ChrisTrenkamp/goxpath v0.0.0-20210404020558-97928f7e12b6 // indirect github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.13.11 // indirect github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.41 // indirect @@ -163,7 +163,7 @@ require ( github.com/gobwas/glob v0.2.3 // indirect github.com/godbus/dbus/v5 v5.0.4 // indirect github.com/gogo/protobuf v1.3.2 // indirect - github.com/golang-jwt/jwt/v5 v5.0.0 // indirect + github.com/golang-jwt/jwt/v5 v5.2.1 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/google/go-cmp v0.6.0 // indirect @@ -224,7 +224,7 @@ require ( github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect github.com/pborman/uuid v1.2.1 // indirect - github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 // indirect + github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pkg/term v1.1.0 // indirect github.com/pkg/xattr v0.4.9 // indirect @@ -234,12 +234,12 @@ require ( github.com/rivo/uniseg v0.4.4 // indirect github.com/robfig/cron/v3 v3.0.1 // indirect github.com/rogpeppe/fastuuid v1.2.0 // indirect - github.com/rogpeppe/go-internal v1.10.0 // indirect + github.com/rogpeppe/go-internal v1.12.0 // indirect github.com/rs/xid v1.5.0 // indirect github.com/sirupsen/logrus v1.9.3 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/std-uritemplate/std-uritemplate/go v0.0.47 // indirect - github.com/stretchr/testify v1.8.4 // indirect + github.com/stretchr/testify v1.9.0 // indirect github.com/vishvananda/netns v0.0.4 // indirect github.com/xdg-go/stringprep v1.0.3 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect @@ -256,7 +256,7 @@ require ( golang.org/x/time v0.5.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20231120223509-83a465c0220f // indirect google.golang.org/grpc v1.59.0 // indirect - google.golang.org/protobuf v1.31.0 // indirect + google.golang.org/protobuf v1.33.0 // indirect gopkg.in/errgo.v1 v1.0.1 // indirect gopkg.in/gobwas/glob.v0 v0.2.3 // indirect gopkg.in/inf.v0 v0.9.1 // indirect diff --git a/go.sum b/go.sum index 95da6368bf7..f825f622ce8 100644 --- a/go.sum +++ b/go.sum @@ -24,12 +24,12 @@ cloud.google.com/go/storage v1.0.0/go.mod h1:IhtSnM/ZTZV8YYJWCY8RULGVqBDmpoyjwiy cloud.google.com/go/storage v1.5.0/go.mod h1:tpKbwo567HUNpVclU5sGELwQWBDZ8gh0ZeosJ0Rtdos= cloud.google.com/go/storage v1.6.0/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohlUTyfDhBk= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= -github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.0 h1:fb8kj/Dh4CSwgsOzHeZY4Xh68cFVbzXx+ONXGMY//4w= -github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.0/go.mod h1:uReU2sSxZExRPBAg3qKzmAucSi51+SP1OhohieR821Q= -github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.4.0 h1:BMAjVKJM0U/CYF27gA0ZMmXGkOcvfFtD0oHVZ1TIPRI= -github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.4.0/go.mod h1:1fXstnBMas5kzG+S3q8UoJcmyU6nUeunJcMDHcRYHhs= -github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.0 h1:d81/ng9rET2YqdVkVwkb6EXeRrLJIwyGnJcAlAWKwhs= -github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.0/go.mod h1:s4kgfzA0covAXNicZHDMN58jExvcng2mC/DepXiF1EI= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.12.0 h1:1nGuui+4POelzDwI7RG56yfQJHCnKvwfMoU7VsEp+Zg= +github.com/Azure/azure-sdk-for-go/sdk/azcore v1.12.0/go.mod h1:99EvauvlcJ1U06amZiksfYz/3aFGyIhWGHVyiZXtBAI= +github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.6.0 h1:U2rTu3Ef+7w9FHKIAXM6ZyqF3UOWJZ12zIm8zECAFfg= +github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.6.0/go.mod h1:9kIvujWAA58nmPmWB1m23fyWic1kYZMxD9CxaWn4Qpg= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.9.0 h1:H+U3Gk9zY56G3u872L82bk4thcsy2Gghb9ExT4Zvm1o= +github.com/Azure/azure-sdk-for-go/sdk/internal v1.9.0/go.mod h1:mgrmMSgaLp9hmax62XQTd0N4aAqSE5E0DulSpVYK7vc= github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v3 v3.0.0-beta.2 h1:qiir/pptnHqp6hV8QwV+IExYIf6cPsXBfUDUXQ27t2Y= github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v3 v3.0.0-beta.2/go.mod h1:jVRrRDLCOuif95HDYC23ADTMlvahB7tMdl519m9Iyjc= github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute v1.0.0 h1:/Di3vB4sNeQ+7A8efjUVENvyB945Wruvstucqp7ZArg= @@ -66,8 +66,8 @@ github.com/Azure/go-autorest/logger v0.2.0/go.mod h1:T9E3cAhj2VqvPOtCYAvby9aBXkZ github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBpUA79WCAKPPZVC2DeU= github.com/Azure/go-ntlmssp v0.0.0-20211209120228-48547f28849e h1:ZU22z/2YRFLyf/P4ZwUYSdNCWsMEI0VeyrFoI2rAhJQ= github.com/Azure/go-ntlmssp v0.0.0-20211209120228-48547f28849e/go.mod h1:chxPXzSsl7ZWRAuOIE23GDNzjWuZquvFlgA8xmpunjU= -github.com/AzureAD/microsoft-authentication-library-for-go v1.1.1 h1:WpB/QDNLpMw72xHJc34BNNykqSOeEJDAWkhf0u12/Jk= -github.com/AzureAD/microsoft-authentication-library-for-go v1.1.1/go.mod h1:wP83P5OoQ5p6ip3ScPr0BAq0BvuPAvacpEuSzyouqAI= +github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 h1:XHOnouVk1mxXfQidrMEnLlPk9UMeRtyBTnEFtxkV0kU= +github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2/go.mod h1:wP83P5OoQ5p6ip3ScPr0BAq0BvuPAvacpEuSzyouqAI= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/toml v1.1.0 h1:ksErzDEI1khOiGPgpwuI7x2ebx/uXQNw7xJpn9Eq1+I= github.com/BurntSushi/toml v1.1.0/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= @@ -285,8 +285,8 @@ github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7a github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q= -github.com/golang-jwt/jwt/v5 v5.0.0 h1:1n1XNM9hk7O9mnQoNBGolZvzebBQ7p93ULHRc28XJUE= -github.com/golang-jwt/jwt/v5 v5.0.0/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= +github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk= +github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/groupcache v0.0.0-20160516000752-02826c3e7903/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= @@ -358,8 +358,8 @@ github.com/google/uuid v0.0.0-20170306145142-6a5e28554805/go.mod h1:TIyPZe4Mgqvf github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/google/uuid v1.4.0 h1:MtMxsa51/r9yyhkyLsVeVt0B+BGQZzpQiTQ4eHZ8bc4= -github.com/google/uuid v1.4.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= +github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/googleapis/enterprise-certificate-proxy v0.3.2 h1:Vie5ybvEvT75RniqhfFxPRy3Bf7vr3h0cechB90XaQs= github.com/googleapis/enterprise-certificate-proxy v0.3.2/go.mod h1:VLSiSSBs/ksPL8kq3OBOQ6WRI2QnaFynd1DCjZ62+V0= github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= @@ -372,8 +372,8 @@ github.com/gorilla/handlers v1.3.0 h1:tsg9qP3mjt1h4Roxp+M1paRjrVBfPSOpBuVclh6Ylu github.com/gorilla/handlers v1.3.0/go.mod h1:Qkdc/uu4tH4g6mTK6auzZ766c4CA0Ng8+o/OAirnOIQ= github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= -github.com/gorilla/schema v1.2.0 h1:YufUaxZYCKGFuAq3c96BOhjgd5nmXiOY9NGzF247Tsc= -github.com/gorilla/schema v1.2.0/go.mod h1:kgLaKoK1FELgZqMAVxx/5cbj0kT+57qxUrAlIO2eleU= +github.com/gorilla/schema v1.4.1 h1:jUg5hUjCSDZpNGLuXQOgIWGdlgrIdYvgQ0wZtdK1M3E= +github.com/gorilla/schema v1.4.1/go.mod h1:Dg5SSm5PV60mhF2NFaTV1xuYYj8tV8NOPRo4FggUMnM= github.com/gorilla/securecookie v1.1.1 h1:miw7JPhV+b/lAHSXz4qd/nN9jRiAFV5FwjeKyCS8BvQ= github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4= github.com/gorilla/sessions v1.2.1 h1:DHd3rPN5lE3Ts3D8rKkQ8x/0kqfeNmBAaiSi+o7FsgI= @@ -783,8 +783,8 @@ github.com/pborman/uuid v1.2.1 h1:+ZZIw58t/ozdjRaXh/3awHfmWRbzYxJoAdNJxe/3pvw= github.com/pborman/uuid v1.2.1/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= -github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU= -github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI= +github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ= +github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU= github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -842,8 +842,8 @@ github.com/rogpeppe/fastuuid v1.2.0 h1:Ppwyp6VYCF1nvBTXL3trRso7mXMlRrw9ooo375wvi github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= -github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= -github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog= +github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= +github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= github.com/rs/cors v1.9.0 h1:l9HGsTsHJcvW14Nk7J9KFz8bzeAWXn3CG6bgt7LsrAE= github.com/rs/cors v1.9.0/go.mod h1:XyqrcTp5zjWr1wsJ8PIRZssZ8b/WMcMf71DJnit4EMU= github.com/rs/xid v1.5.0 h1:mKX4bl4iPYJtEIxp6CYiUuLQ/8DYMoz0PUdtGgMFRVc= @@ -880,8 +880,9 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= -github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= @@ -891,8 +892,8 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= -github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= +github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= @@ -1107,7 +1108,6 @@ golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210426230700-d19ff857e887/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210806184541-e5e7981a1069/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -1115,6 +1115,7 @@ golang.org/x/sys v0.0.0-20220408201424-a24fb2fb8a0f/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= @@ -1252,8 +1253,8 @@ google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGj google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= -google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs8= -google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= +google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI= +google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20160105164936-4f90aeace3a2/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= From b9489dd84f2d9a431fb4c06cfdd9706d976bacbb Mon Sep 17 00:00:00 2001 From: Harry Pidcock Date: Tue, 23 Jul 2024 14:23:23 +1000 Subject: [PATCH 14/28] ci: add govulncheck + backported cleanup --- .github/workflows/static-analysis.yml | 7 ++----- acceptancetests/jujupy/client.py | 10 +++++----- acceptancetests/jujupy/tests/test_status.py | 2 +- acceptancetests/repository/trusty/haproxy/cm.py | 4 ++-- acceptancetests/tests/test_jujucharm.py | 2 +- scripts/find-bad-doc-comments.py | 4 ++-- scripts/leadershipclaimer/count-leadership.py | 4 ++-- tests/README.md | 2 +- tests/suites/static_analysis/lint_go.sh | 15 +++++++++++++-- 9 files changed, 29 insertions(+), 21 deletions(-) diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index e13aa2d7192..269920f043a 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -49,7 +49,8 @@ jobs: echo "GOPATH=$(go env GOPATH)" >> $GITHUB_ENV echo "$(go env GOPATH)/bin" >> $GITHUB_PATH - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.54.2 + go install golang.org/x/vuln/cmd/govulncheck@latest + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.59.1 sudo curl -sSfL https://github.com/mvdan/sh/releases/download/v3.7.0/shfmt_v3.7.0_linux_$(go env GOARCH) -o /usr/bin/shfmt sudo chmod +x /usr/bin/shfmt sudo DEBIAN_FRONTEND=noninteractive apt install -y expect @@ -58,19 +59,16 @@ jobs: run: go mod download - name: "Static Analysis: Copyright" - if: steps.filter.outputs.static-analysis == 'true' || steps.filter.outputs.go == 'true' run: | STATIC_ANALYSIS_JOB=test_copyright make static-analysis shell: bash - name: "Static Analysis: Shell Check" - if: steps.filter.outputs.static-analysis == 'true' || steps.filter.outputs.sh == 'true' run: | STATIC_ANALYSIS_JOB=test_static_analysis_shell make static-analysis shell: bash - name: "Static Analysis: Go Check" - if: steps.filter.outputs.static-analysis == 'true' || steps.filter.outputs.go == 'true' run: | # Explicitly set GOROOT to avoid golangci-lint/issues/3107 export "GOROOT=$(go env GOROOT)" @@ -78,7 +76,6 @@ jobs: shell: bash - name: "Static Analysis: Python Check" - if: steps.filter.outputs.static-analysis == 'true' || steps.filter.outputs.python == 'true' run: | STATIC_ANALYSIS_JOB=test_static_analysis_python make static-analysis shell: bash diff --git a/acceptancetests/jujupy/client.py b/acceptancetests/jujupy/client.py index bc9284fc094..66e57c17ef0 100644 --- a/acceptancetests/jujupy/client.py +++ b/acceptancetests/jujupy/client.py @@ -1868,7 +1868,7 @@ def backup(self): raise log.info('backup file {}'.format(output)) backup_file_pattern = re.compile( - '(juju-backup-[0-9-]+\.(t|tar.)gz)'.encode('ascii')) + '(juju-backup-[0-9-]+\\.(t|tar.)gz)'.encode('ascii')) match = backup_file_pattern.search(output) if match is None: raise Exception("The backup file was not found in output: %s" % @@ -2273,7 +2273,7 @@ def handle_openstack(self, child, cloud): child.expect(self.REGION_ENDPOINT_PROMPT) child.sendline(values['endpoint']) match = child.expect([ - u"Enter another region\? \([yY]/[nN]\):", + u"Enter another region\\? \\([yY]/[nN]\\):", u"Can't validate endpoint" ]) if match == 1: @@ -2285,7 +2285,7 @@ def handle_openstack(self, child, cloud): def handle_vsphere(self, child, cloud): match = child.expect([u"Enter a name for your .* cloud:", - u'Enter the (vCenter address or URL|API endpoint url for the cloud \[\]):']) + u'Enter the (vCenter address or URL|API endpoint url for the cloud \\[\\]):']) if match == 0: raise NameNotAccepted('Cloud name not accepted.') if match == 1: @@ -2301,7 +2301,7 @@ def handle_vsphere(self, child, cloud): raise InvalidEndpoint() child.sendline(name) child.expect( - u'Enter another (datacenter|region)\? \([yY]/[nN]\):') + u'Enter another (datacenter|region)\\? \\([yY]/[nN]\\):') if num + 1 < len(cloud['regions']): child.sendline('y') else: @@ -2410,7 +2410,7 @@ def register_user_interactively(client, token, controller_name): child.sendline(password) child.expect(u'Confirm password:') child.sendline(password) - child.expect(u'Enter a name for this controller( \[.*\])?:') + child.expect(u'Enter a name for this controller( \\[.*\\])?:') child.sendline(controller_name) def login_if_need(session): diff --git a/acceptancetests/jujupy/tests/test_status.py b/acceptancetests/jujupy/tests/test_status.py index 0c0efae7ec3..9c8bca35082 100644 --- a/acceptancetests/jujupy/tests/test_status.py +++ b/acceptancetests/jujupy/tests/test_status.py @@ -80,7 +80,7 @@ def test_to_exception_stuck_allocating(self): current='allocating', message='foo') with self.assertRaisesRegexp( StuckAllocatingError, - "\('0', 'Stuck allocating. Last message: foo'\)"): + "\\('0', 'Stuck allocating. Last message: foo'\\)"): raise item.to_exception() def test_to_exception_allocating_unit(self): diff --git a/acceptancetests/repository/trusty/haproxy/cm.py b/acceptancetests/repository/trusty/haproxy/cm.py index f728b8608f4..72775f991f5 100644 --- a/acceptancetests/repository/trusty/haproxy/cm.py +++ b/acceptancetests/repository/trusty/haproxy/cm.py @@ -32,7 +32,7 @@ def get_branch_config(config_file): line = line.split('#')[0].strip() bzr_match = re.match(r'(\S+)\s+' 'lp:([^;]+)' - '(?:;revno=(\d+))?', line) + '(?:;revno=(\\d+))?', line) if bzr_match: name, branch, revno = bzr_match.group(1, 2, 3) if revno is None: @@ -42,7 +42,7 @@ def get_branch_config(config_file): branches[name] = (branch, revspec) continue dir_match = re.match(r'(\S+)\s+' - '\(directory\)', line) + '\\(directory\\)', line) if dir_match: name = dir_match.group(1) branches[name] = None diff --git a/acceptancetests/tests/test_jujucharm.py b/acceptancetests/tests/test_jujucharm.py index f7e9c79732f..ffc7a5e7231 100644 --- a/acceptancetests/tests/test_jujucharm.py +++ b/acceptancetests/tests/test_jujucharm.py @@ -118,7 +118,7 @@ def test_ensure_valid_name(self): self.assertIsNone(Charm.NAME_REGEX.match(charm.metadata['name'])) self.assertRaisesRegexp( JujuAssertionError, - 'Invalid Juju Charm Name, "BAD_NAME" does not match ".*"\.', + 'Invalid Juju Charm Name, "BAD_NAME" does not match ".*"\\.', Charm, 'BAD_NAME', 'A charm with a checked bad name') def test_ensure_valid_name_anchoring(self): diff --git a/scripts/find-bad-doc-comments.py b/scripts/find-bad-doc-comments.py index 42bdee56536..196ca628459 100755 --- a/scripts/find-bad-doc-comments.py +++ b/scripts/find-bad-doc-comments.py @@ -31,8 +31,8 @@ def find_go_files(root): yield path.join(directory, filename) DOC_COMMENT_PATT = '\n\n//.+\n(//.+\n)*func.+\n' -FIRST_WORD_PATT = '// *(\w+)' -FUNC_NAME_PATT = 'func(?: \([^)]+\))? (\S+)\(' +FIRST_WORD_PATT = '// *(\\w+)' +FUNC_NAME_PATT = 'func(?: \\([^)]+\\))? (\\S+)\\(' def extract_doc_comments(text): for match in re.finditer(DOC_COMMENT_PATT, text, re.MULTILINE): diff --git a/scripts/leadershipclaimer/count-leadership.py b/scripts/leadershipclaimer/count-leadership.py index ddc66e425f0..c872213815b 100644 --- a/scripts/leadershipclaimer/count-leadership.py +++ b/scripts/leadershipclaimer/count-leadership.py @@ -13,9 +13,9 @@ def main(args): p.add_argument("--tick", type=float, default=1.0, help="seconds between printing status ticks") opts = p.parse_args(args) - actionsRE = re.compile("\s*(?P