Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Catch readonly-prohibited operations early
Browse files Browse the repository at this point in the history
Operations that need to write back to the git repo will fail
eventually if they try to push changes upstream; but this can look a
bit mysterious to the end user, and ought to just be a safety net.

This commit adds some mechanisms for failing such operations earlier
and more gracefully, when running with readonly mode:

 - distinguish between clones needed for reads, and clones needed for
   writes, and for the latter case,
   - make working (writable) clones of a read-only repo fail (again)
 - mark all workloads as read-only in ListServices* responses, when
   the repo is read-only
 - skip automated image updates, when the repo is read-only
  • Loading branch information
squaremo committed Aug 14, 2019
1 parent df653ef commit 678d008
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 14 deletions.
1 change: 1 addition & 0 deletions api/v6/api.go
Expand Up @@ -28,6 +28,7 @@ const (
ReadOnlySystem ReadOnlyReason = "System"
ReadOnlyNoRepo ReadOnlyReason = "NoRepo"
ReadOnlyNotReady ReadOnlyReason = "NotReady"
ReadOnlyROMode ReadOnlyReason = "ReadonlyMode"
)

type ControllerStatus struct {
Expand Down
45 changes: 40 additions & 5 deletions daemon/daemon.go
Expand Up @@ -79,6 +79,15 @@ type repo interface {
AbsolutePaths() []string
}

type exportRepo struct {
*git.Export
paths []string
}

func (r exportRepo) AbsolutePaths() []string {
return git.MakeAbsolutePaths(r, r.paths)
}

func (d *Daemon) getManifestStore(r repo) (manifests.Store, error) {
if d.ManifestGenerationEnabled {
return manifests.NewConfigAware(r.Dir(), r.AbsolutePaths(), d.Manifests)
Expand All @@ -89,8 +98,9 @@ func (d *Daemon) getManifestStore(r repo) (manifests.Store, error) {
func (d *Daemon) getResources(ctx context.Context) (map[string]resource.Resource, v6.ReadOnlyReason, error) {
var resources map[string]resource.Resource
var globalReadOnly v6.ReadOnlyReason
err := d.WithClone(ctx, func(checkout *git.Checkout) error {
cm, err := d.getManifestStore(checkout)
err := d.WithReadonlyClone(ctx, func(checkout *git.Export) error {
r := exportRepo{checkout, d.GitConfig.Paths}
cm, err := d.getManifestStore(r)
if err != nil {
return err
}
Expand Down Expand Up @@ -143,13 +153,17 @@ func (d *Daemon) ListServicesWithOptions(ctx context.Context, opts v11.ListServi
var res []v6.ControllerStatus
for _, workload := range clusterWorkloads {
readOnly := v6.ReadOnlyOK
repoIsReadonly := d.Repo.Readonly()

var policies policy.Set
if resource, ok := resources[workload.ID.String()]; ok {
policies = resource.Policies()
}
switch {
case policies == nil:
readOnly = missingReason
case repoIsReadonly:
readOnly = v6.ReadOnlyROMode
case workload.IsSystem:
readOnly = v6.ReadOnlySystem
}
Expand Down Expand Up @@ -251,7 +265,7 @@ type updateFunc func(ctx context.Context, jobID job.ID, working *git.Checkout, l
func (d *Daemon) makeJobFromUpdate(update updateFunc) jobFunc {
return func(ctx context.Context, jobID job.ID, logger log.Logger) (job.Result, error) {
var result job.Result
err := d.WithClone(ctx, func(working *git.Checkout) error {
err := d.WithWorkingClone(ctx, func(working *git.Checkout) error {
var err error
if err = verifyWorkingRepo(ctx, d.Repo, working, d.SyncState); d.GitVerifySignatures && err != nil {
return err
Expand Down Expand Up @@ -567,7 +581,7 @@ func (d *Daemon) JobStatus(ctx context.Context, jobID job.ID) (job.Status, error
// means that even if fluxd restarts, we will at least remember
// jobs which have pushed a commit.
// FIXME(michael): consider looking at the repo for this, since read op
err := d.WithClone(ctx, func(working *git.Checkout) error {
err := d.WithWorkingClone(ctx, func(working *git.Checkout) error {
notes, err := working.NoteRevList(ctx)
if err != nil {
return errors.Wrap(err, "enumerating commit notes")
Expand Down Expand Up @@ -648,7 +662,12 @@ func (d *Daemon) GitRepoConfig(ctx context.Context, regenerate bool) (v6.GitConf

// Non-api.Server methods

func (d *Daemon) WithClone(ctx context.Context, fn func(*git.Checkout) error) error {
// WithWorkingClone applies the given func to a fresh, writable clone
// of the git repo, and cleans it up afterwards. This may return an
// error in the case that the repo is read-only; use
// `WithReadonlyClone` if you only need to read the files in the git
// repo.
func (d *Daemon) WithWorkingClone(ctx context.Context, fn func(*git.Checkout) error) error {
co, err := d.Repo.Clone(ctx, d.GitConfig)
if err != nil {
return err
Expand All @@ -657,6 +676,22 @@ func (d *Daemon) WithClone(ctx context.Context, fn func(*git.Checkout) error) er
return fn(co)
}

// WithReadonlyClone applies the given func to an export of the
// current revision of the git repo. Use this if you just need to
// consult the files.
func (d *Daemon) WithReadonlyClone(ctx context.Context, fn func(*git.Export) error) error {
head, err := d.Repo.BranchHead(ctx)
if err != nil {
return err
}
co, err := d.Repo.Export(ctx, head)
if err != nil {
return err
}
defer co.Clean()
return fn(co)
}

func (d *Daemon) LogEvent(ev event.Event) error {
if d.EventWriter == nil {
d.Logger.Log("event", ev, "logupstream", "false")
Expand Down
12 changes: 12 additions & 0 deletions daemon/loop.go
Expand Up @@ -52,6 +52,13 @@ func (d *Daemon) Loop(stop chan struct{}, wg *sync.WaitGroup, logger log.Logger)
// In-memory sync tag state
ratchet := &lastKnownSyncState{logger: logger, state: d.SyncState}

// If the git repo is read-only, the image update will fail; to
// avoid repeated failures in the log, mention it here and
// otherwise skip it when it comes around.
if d.Repo.Readonly() {
logger.Log("info", "Repo is read-only; no image updates will be attempted")
}

// Ask for a sync, and to check
d.AskForSync()
d.AskForAutomatedWorkloadImageUpdates()
Expand All @@ -68,6 +75,11 @@ func (d *Daemon) Loop(stop chan struct{}, wg *sync.WaitGroup, logger log.Logger)
default:
}
}
if d.Repo.Readonly() {
// don't bother trying to update images, and don't
// bother setting the timer again
continue
}
d.pollForNewAutomatedWorkloadImages(logger)
automatedWorkloadTimer.Reset(d.AutomationInterval)
case <-automatedWorkloadTimer.C:
Expand Down
6 changes: 6 additions & 0 deletions git/repo.go
Expand Up @@ -127,6 +127,12 @@ func (r *Repo) Origin() Remote {
return r.origin
}

// Readonly returns `true` if the repo was marked as readonly, `false`
// otherwise
func (r *Repo) Readonly() bool {
return r.readonly
}

// Dir returns the local directory into which the repo has been
// cloned, if it has been cloned.
func (r *Repo) Dir() string {
Expand Down
28 changes: 19 additions & 9 deletions git/working.go
Expand Up @@ -59,6 +59,9 @@ type TagAction struct {
// Clone returns a local working clone of the sync'ed `*Repo`, using
// the config given.
func (r *Repo) Clone(ctx context.Context, conf Config) (*Checkout, error) {
if r.readonly {
return nil, ErrReadOnly
}
upstream := r.Origin()
repoDir, err := r.workingClone(ctx, conf.Branch)
if err != nil {
Expand Down Expand Up @@ -112,21 +115,28 @@ func (r *Repo) Clone(ctx context.Context, conf Config) (*Checkout, error) {
}, nil
}

// AbsolutePaths returns the absolute paths as configured. It ensures
// that at least one path is returned, so that it can be used with
// `Manifest.LoadManifests`.
func (c *Checkout) AbsolutePaths() []string {
if len(c.config.Paths) == 0 {
return []string{c.Dir()}
// MakeAbsolutePaths returns the absolute path for each of the
// relativePaths given, taking the repo's location as the base.
func MakeAbsolutePaths(r interface{ Dir() string }, relativePaths []string) []string {
if len(relativePaths) == 0 {
return []string{r.Dir()}
}

paths := make([]string, len(c.config.Paths), len(c.config.Paths))
for i, p := range c.config.Paths {
paths[i] = filepath.Join(c.Dir(), p)
base := r.Dir()
paths := make([]string, len(relativePaths), len(relativePaths))
for i, p := range relativePaths {
paths[i] = filepath.Join(base, p)
}
return paths
}

// AbsolutePaths returns the absolute paths as configured. It ensures
// that at least one path is returned, so that it can be used with
// `Manifest.LoadManifests`.
func (c *Checkout) AbsolutePaths() []string {
return MakeAbsolutePaths(c, c.config.Paths)
}

// CommitAndPush commits changes made in this checkout, along with any
// extra data as a note, and pushes the commit and note to the remote repo.
func (c *Checkout) CommitAndPush(ctx context.Context, commitAction CommitAction, note interface{}, addUntracked bool) error {
Expand Down

0 comments on commit 678d008

Please sign in to comment.