Skip to content

Commit

Permalink
Construct charm origin from charm url
Browse files Browse the repository at this point in the history 
Where possible, we should construct charm origin using the charm url.
Some versions of the Juju client/server do not properly process/validate
charm origins, meaning it's possible for get incorrect data into this
field

Do this construction during upgrades and migrations
  • Loading branch information
@jack-w-shaw
jack-w-shaw committed Oct 31, 2023
1 parent ff13c80 commit aaa2477
Show file tree
Hide file tree
Showing 7 changed files with 222 additions and 59 deletions.
2 changes: 1 addition & 1 deletion state/export_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ func AddTestingCharmWithSeries(c *gc.C, st *State, name string, series string) *
}

func getCharmRepo(series string) *repo.CharmRepo {
// ALl testing charms for state are under `quantal` except `kubernetes`.
// All testing charms for state are under `quantal` except `kubernetes`.
if series == "kubernetes" {
return testcharms.RepoForSeries(series)
}
Expand Down
92 changes: 59 additions & 33 deletions state/migration_import.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1407,35 +1407,46 @@ func (i *importer) makeApplicationDoc(a description.Application) (*applicationDo
return appDoc, nil
}

// makeCharmOrigin returns the charm origin for an application
//
// Previous versions of the Juju server and clients have treated applications charm
// origins very loosely, particularly during `refresh -- switch`s. The server performed
// no validation on origins received from the client, and client often mutated them
// incorrectly. For instance, when switching from a ch charm to local, pylibjuju simply
// send back a copy of the ch charm origin, whereas the CLI only set the source to local.
// Both resulted in incorrect/invalidate origins.
//
// Calculate the origin Source and Revision from the charm url. Ensure ID, Hash and Channel
// are dropped from local charm. Keep ID, Hash and Channel (for ch charms) and Platform (always)
// we get from the origin. We can trust these since supported clients cannot break these
//
// This was fixed in pylibjuju 3.2.3.0 and juju 3.3.0. As of writing, no versions of the
// server validate new charm origins on calls to SetCharm. Ideally, the client shouldn't
// handle charm origins at all, being an implementation detail. But this will probably have
// to wait until the api re-write
//
// https://bugs.launchpad.net/juju/+bug/2039267
// https://github.com/juju/python-libjuju/issues/962
//
// TODO: Once we have confidence in charm origins, do not parse charm url and simplify
// into a translation layer
func (i *importer) makeCharmOrigin(a description.Application) (*CharmOrigin, error) {
curlStr := a.CharmURL()
sourceOrigin := a.CharmOrigin()
curl, err := charm.ParseURL(a.CharmURL())
if err != nil {
return nil, errors.Trace(err)
}

// Fix bad datasets from LP 1999060 during migration.
// ID and Hash missing from N-1 of N applications'
// charm origins when deployed using the same charm.
if foundOrigin, ok := i.charmOrigins[curlStr]; ok {
if foundOrigin, ok := i.charmOrigins[curl.String()]; ok {
return foundOrigin, nil
}

co := a.CharmOrigin()
rev := co.Revision()
// If revision is empty we export revision -1. In this case
// parse the revision from the url
// NOTE: We use <= 0 because some old versions of juju default
// revision to 0 when it's empty
if rev <= 0 {
curl, err := charm.ParseURL(a.CharmURL())
if err != nil {
return nil, errors.Trace(err)
}
rev = curl.Revision
}

var channel *Channel
// Only charmhub charms will have a channel. Local charms
// should not have a channel, so drop even if provided.
serialized := co.Channel()
if serialized != "" && corecharm.CharmHub.Matches(co.Source()) {
serialized := sourceOrigin.Channel()
if serialized != "" && charm.CharmHub.Matches(curl.Schema) {
c, err := charm.ParseChannelNormalize(serialized)
if err != nil {
return nil, errors.Trace(err)
Expand All @@ -1449,11 +1460,9 @@ func (i *importer) makeCharmOrigin(a description.Application) (*CharmOrigin, err
Risk: string(c.Risk),
Branch: c.Branch,
}
} else if serialized != "" && corecharm.Local.Matches(co.Source()) {
i.logger.Warningf("Dropping channel: %q for application %q, should not exist", serialized, a.Name())
}

p, err := corecharm.ParsePlatformNormalize(co.Platform())
p, err := corecharm.ParsePlatformNormalize(sourceOrigin.Platform())
if err != nil {
return nil, errors.Trace(err)
}
Expand All @@ -1464,16 +1473,33 @@ func (i *importer) makeCharmOrigin(a description.Application) (*CharmOrigin, err
}

// We can hardcode type to charm as we never store bundles in state.
origin := &CharmOrigin{
Source: co.Source(),
Type: "charm",
ID: co.ID(),
Hash: co.Hash(),
Revision: &rev,
Channel: channel,
Platform: platform,
}
i.charmOrigins[curlStr] = origin
var origin *CharmOrigin
if charm.Local.Matches(curl.Schema) {
origin = &CharmOrigin{
Source: corecharm.Local.String(),
Type: "charm",
Revision: &curl.Revision,
Platform: platform,
}
} else if charm.CharmHub.Matches(curl.Schema) {
origin = &CharmOrigin{
Source: corecharm.CharmHub.String(),
Type: "charm",
Revision: &curl.Revision,
ID: sourceOrigin.ID(),
Hash: sourceOrigin.Hash(),
Channel: channel,
Platform: platform,
}
} else {
return nil, errors.Errorf("Unrecognised charm url schema %q", curl.Schema)
}

if !reflect.DeepEqual(sourceOrigin, origin) {
i.logger.Warningf("Source origin for application %q does not match charm url. Normalising", a.Name())
}

i.charmOrigins[curl.String()] = origin
return origin, nil
}

Expand Down
56 changes: 52 additions & 4 deletions state/migration_import_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,8 @@ func (s *MigrationImportSuite) setupSourceApplications(
Channel: &state.Channel{
Risk: "edge",
},
Hash: "some-hash",
ID: "some-id",
Platform: platform,
},
CharmConfig: map[string]interface{}{
Expand Down Expand Up @@ -697,25 +699,71 @@ func (s *MigrationImportSuite) TestApplicationsUpdateSeriesNotPlatform(c *gc.C)
c.Assert(obtainedOrigin.Platform.Channel, gc.Equals, "20.04/stable")
}

func (s *MigrationImportSuite) TestLocalApplicationCharmOriginRevisionFilledIn(c *gc.C) {
func (s *MigrationImportSuite) TestCharmhubApplicationCharmOriginNormalised(c *gc.C) {
platform := &state.Platform{Architecture: corearch.DefaultArchitecture, OS: "ubuntu", Channel: "12.10/stable"}
f := factory.NewFactory(s.State, s.StatePool)

testCharm := f.MakeCharm(c, &factory.CharmParams{Revision: "8"})
testCharm := f.MakeCharm(c, &factory.CharmParams{Revision: "8", URL: "ch:mysql-8"})
wrongRev := 4
_ = f.MakeApplication(c, &factory.ApplicationParams{
Charm: testCharm,
Name: "mysql",
CharmOrigin: &state.CharmOrigin{
Source: "local",
Source: "charm-hub",
Type: "charm",
Platform: platform,
Revision: &wrongRev,
Channel: &state.Channel{Track: "20.04", Risk: "stable", Branch: "deadbeef"},
Hash: "some-hash",
ID: "some-id",
},
})

_, newSt := s.importModel(c, s.State)
newApp, err := newSt.Application("mysql")
c.Assert(err, jc.ErrorIsNil)
rev := 8
c.Assert(newApp.CharmOrigin(), gc.DeepEquals, &state.CharmOrigin{
Source: "charm-hub",
Type: "charm",
Platform: platform,
Revision: &rev,
Channel: &state.Channel{Track: "20.04", Risk: "stable", Branch: "deadbeef"},
Hash: "some-hash",
ID: "some-id",
})
}

func (s *MigrationImportSuite) TestLocalApplicationCharmOriginNormalised(c *gc.C) {
platform := &state.Platform{Architecture: corearch.DefaultArchitecture, OS: "ubuntu", Channel: "12.10/stable"}
f := factory.NewFactory(s.State, s.StatePool)

testCharm := f.MakeCharm(c, &factory.CharmParams{Revision: "8", URL: "local:mysql-8"})
wrongRev := 4
_ = f.MakeApplication(c, &factory.ApplicationParams{
Charm: testCharm,
Name: "mysql",
CharmOrigin: &state.CharmOrigin{
Source: "charm-hub",
Type: "charm",
Platform: platform,
Revision: &wrongRev,
Channel: &state.Channel{Track: "20.04", Risk: "stable", Branch: "deadbeef"},
Hash: "some-hash",
ID: "some-id",
},
})

_, newSt := s.importModel(c, s.State)
newApp, err := newSt.Application("mysql")
c.Assert(err, jc.ErrorIsNil)
c.Assert(*newApp.CharmOrigin().Revision, gc.Equals, 8)
rev := 8
c.Assert(newApp.CharmOrigin(), gc.DeepEquals, &state.CharmOrigin{
Source: "local",
Type: "charm",
Platform: platform,
Revision: &rev,
})
}

func (s *MigrationImportSuite) TestApplicationStatus(c *gc.C) {
Expand Down
52 changes: 44 additions & 8 deletions state/upgrades.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/juju/mgo/v3/txn"
"github.com/juju/names/v4"

corecharm "github.com/juju/juju/core/charm"
"github.com/juju/juju/core/network"
"github.com/juju/juju/core/secrets"
"github.com/juju/juju/mongo"
Expand Down Expand Up @@ -255,7 +256,30 @@ func EnsureInitalRefCountForExternalSecretBackends(pool *StatePool) error {
return nil
}

func EnsureApplicationCharmOriginsHaveRevisions(pool *StatePool) error {
// EnsureApplicationCharmOriginsNormalised fixes application charm origins which may
// have been broken
//
// Previous versions of the Juju server and clients have treated applications charm
// origins very loosely, particularly during `refresh -- switch`s. The server performed
// no validation on origins received from the client, and client often mutated them
// incorrectly. For instance, when switching from a ch charm to local, pylibjuju simply
// send back a copy of the ch charm origin, whereas the CLI only set the source to local.
// Both resulted in incorrect/invalidate origins.
//
// Calculate the origin Source and Revision from the charm url. Ensure ID, Hash and Channel
// are dropped from local charm. Keep ID, Hash and Channel (for ch charms) and Platform (always)
// we get from the origin. We can trust these since supported clients cannot break these
//
// This was fixed in pylibjuju 3.2.3.0 and juju 3.3.0. As of writing, no versions of the
// server validate new charm origins on calls to SetCharm. Ideally, the client shouldn't
// handle charm origins at all, being an implementation detail. But this will probably have
// to wait until the api re-write
//
// https://bugs.launchpad.net/juju/+bug/2039267
// https://github.com/juju/python-libjuju/issues/962
//
// TODO: Drop this step once we have confidence in our application charm origins
func EnsureApplicationCharmOriginsNormalised(pool *StatePool) error {
return errors.Trace(runForAllModelStates(pool, func(st *State) error {
allApps, err := st.AllApplications()
if err != nil {
Expand All @@ -265,12 +289,9 @@ func EnsureApplicationCharmOriginsHaveRevisions(pool *StatePool) error {
var ops []txn.Op

for _, app := range allApps {
// We only need to fill in the revision if it's nil/
// There is an edge case though where a previous migration
// has incorrectly filled in the revision to 0 or -1
rev := app.CharmOrigin().Revision
if rev != nil && *rev > 0 {
continue
origin := app.CharmOrigin()
if origin == nil {
return errors.Errorf("application %q has no origin", app.Name())
}
curlStr, _ := app.CharmURL()
if curlStr == nil {
Expand All @@ -283,10 +304,25 @@ func EnsureApplicationCharmOriginsHaveRevisions(pool *StatePool) error {
if curl.Revision == -1 {
return errors.Errorf("charm url %q has no revision", curl.String())
}

if charm.Local.Matches(curl.Schema) {
origin.Source = corecharm.Local.String()
origin.Channel = nil
origin.Hash = ""
origin.ID = ""
origin.Revision = &curl.Revision
} else if charm.CharmHub.Matches(curl.Schema) {
origin.Source = corecharm.CharmHub.String()
origin.Revision = &curl.Revision
} else {
return errors.Errorf("Unrecognised schema charm url schema %q", curl.Schema)
}

ops = append(ops, txn.Op{
C: applicationsC,
Id: app.doc.DocID,
Update: bson.D{{"$set", bson.D{{"charm-origin.revision", curl.Revision}}}},
Assert: txn.DocExists,
Update: bson.D{{"$set", bson.D{{"charm-origin", origin}}}},
})
}
if len(ops) > 0 {
Expand Down
Loading

0 comments on commit aaa2477

Please sign in to comment.