feat: handle key rotation in plain pull, push and diff behind features.key_rotation

README.md

@@ -36,6 +36,8 @@ client:
     - read_configuration # alternative scope that can be used only to read configuration
 storage:
   dir_path: "/tmp/data" # path to local configuration; default: "data"
+features: # optional behaviour of plain pull, push and diff; all default to false
+  key_rotation: false # also handle workspace key rotation, see "Managing key rotation"
 
 profiles: # an optional map of profiles available for use, especially helpful when you want to compare multiple configurations
   stage: # each profile support same configuration as root (aka default profile)
@@ -44,10 +46,19 @@ profiles: # an optional map of profiles available for use, especially helpful wh
       client_id: fb346c287c4d4e378cbae39aa0cxxxxx
       tenant_id: postmance-stage
       client_secret: invalid_secret
+      scopes: # client replaces the top-level client as a whole, so key rotation needs its scopes here too
+        - manage_configuration
+        - manage_servers
     storage:
       dir_path: "/tmp/other"
+    features: # like client and storage, replaces the top-level features for this profile
+      key_rotation: true
 ```
 
+A profile that does not define `client`, `storage`, `logging` or `features` uses the
+top-level section instead. A profile that inherits a top-level `features.key_rotation: true`
+must either set it to `false` or carry the `manage_servers` scope in its own `client.scopes`.
+
 ## Commands
 
 ### Help
@@ -242,12 +253,7 @@ map[string]any{
 ### Managing key rotation
 
 Automatic key rotation settings of a workspace live behind a dedicated API and are
-managed with the `key-rotation` subcommand of `pull`, `push`, and `diff`, for the
-workspace selected with `--workspace` (`--tenant` is rejected). The subcommands do not
-take the parent commands' own flags such as `--filter`, `--method`, or `--source`; only
-`push key-rotation` keeps `--dry-run` and `--out`, and `diff key-rotation` keeps `--out`
-and `--colors`. Plain `pull`, `push`, and `diff` never read or write these settings.
-They are stored in `workspaces/<workspace-id>/key_rotation.yaml`. With several storage
+stored in `workspaces/<workspace-id>/key_rotation.yaml`. With several storage
 directories, the file is taken whole from the first directory that has it; files are
 not merged across directories:
 
@@ -262,6 +268,15 @@ enc:
   cron: "0 0 1 1 *" # required even when disabled
 ```
 
+The plain `pull`, `push`, and `diff` handle the file when
+[`features.key_rotation`](#key-rotation-in-plain-pull-push-and-diff) is enabled. The
+`key-rotation` subcommand of each is the same as the plain command with
+`--filter key_rotation` for the selected workspace, whether or not the feature is
+enabled; `diff key-rotation` compares `local` against `remote`. The subcommands reject
+`--tenant` and do not take the parent commands' own flags such as `--filter`,
+`--method`, or `--source`; only `push key-rotation` keeps `--dry-run` and `--out`, and
+`diff key-rotation` keeps `--out` and `--colors`.
+
 ```bash
 # write the remote settings to workspaces/demo/key_rotation.yaml
 cac --config ./cac.yaml --profile dev --workspace demo pull key-rotation
@@ -276,9 +291,14 @@ cac --config ./cac.yaml --profile dev --workspace demo push key-rotation
 cac --config ./cac.yaml --profile dev --workspace demo diff key-rotation
 ```
 
-The system workspace client used by cac needs the `manage_servers` scope for these
-subcommands, in addition to `manage_configuration`: add it to `client.scopes` in the
-[configuration](#configuration).
+The system workspace client used by cac needs the `manage_servers` scope for key
+rotation, in addition to `manage_configuration`: add it to `client.scopes` in the
+[configuration](#configuration). With `features.key_rotation: true`, cac refuses to start
+without it: `features.key_rotation requires the manage_servers scope in client.scopes`.
+`manage_servers` is broad: it also allows creating and deleting workspaces, and there is
+no read-only alternative today. Every call cac makes in a run uses the same token, so the
+whole run holds that scope. If your main CI client should stay narrow, manage key
+rotation from a dedicated profile with its own client.
 
 Constraints:
 
@@ -293,10 +313,82 @@ Constraints:
   `@hourly` and the `L`, `W`, and `#` modifiers are supported; `@every` is not.
 - `starting_from` is optional and write-only: SecureAuth never returns it, so `pull`
   never writes it and `diff` ignores it. It is only honored when it is in the future.
+- `push` reads the remote settings first and skips a use whose `enabled` and `cron`
+  already match. A use that sets `starting_from` is always sent: SecureAuth never returns
+  it, so cac cannot tell whether it is already applied.
 - `scheduled_at` is read-only and rejected in the file.
 
-> **Note:** `push key-rotation` validates every cron before any API call,
-> and `--dry-run` prints the settings that would be sent instead of pushing them.
+#### Key rotation in plain pull, push and diff
+
+With `features.key_rotation: true`, at the top level or in the selected profile, the
+plain `pull`, `push`, and `diff` handle key rotation too, so a single `cac diff` or
+`cac push`, for example in CI, covers it:
+
+```yaml
+client:
+  scopes:
+    - manage_configuration
+    - manage_servers
+features:
+  key_rotation: true
+```
+
+- Key rotation is handled when `--filter` is not given or includes `key_rotation`.
+  `--filter key_rotation` alone handles only key rotation and skips the rest of the
+  configuration; `--filter clients,key_rotation` handles both.
+- `pull` writes `key_rotation.yaml` after the configuration. With `--tenant` it does so
+  for every workspace of the tenant.
+- `push` reads and validates `key_rotation.yaml` before pushing anything, pushes it after
+  the configuration, and skips it when the file is absent. With `--tenant` it pushes the
+  file of every workspace directory that has one, including stale directories without
+  `server.yaml`: their key rotation request fails with a clear 404 after the import, and
+  rerunning the push is idempotent. `--method` and `--mode` do not apply to it: every key use
+  in the file that differs from the server is replaced.
+- `push --dry-run` writes the settings that would be sent after the configuration. With
+  `--out -` (stdout) or a file, each workspace is a separate YAML document; an `--out`
+  file is emptied first, so it holds only this run's output:
+
+  ```yaml
+  # ... configuration ...
+  ---
+  # key_rotation (demo)
+  sig:
+    enabled: true
+    cron: 0 0 1 * *
+  ```
+
+  With `--out` set to an existing directory, the settings go to
+  `<out>/workspaces/<workspace-id>/key_rotation.yaml`.
+- `diff` appends a section for each workspace whose settings differ, after the
+  configuration diff and in the same format, honoring `--out` and `--colors`:
+
+  ```
+  --- key_rotation (demo)
+    models.Rfc7396PatchOperation{
+    	"sig": map[string]any{
+  - 		"cron":    string("@monthly"),
+  + 		"cron":    string("0 0 1 * *"),
+    		"enabled": bool(true),
+    	},
+    }
+  ```
+
+  `--source` and `--target` map to key rotation as follows: `local` reads
+  `key_rotation.yaml`, `remote` reads the server, and `merged` reads the local file,
+  falling back to the server when the workspace has none. A workspace missing on the
+  target server has no settings there; one missing on the source server fails the diff. With `--tenant` the source's workspaces are compared: the local
+  workspace directories that have the file for `local`, the workspaces of the tenant for
+  `remote`, and both for `merged`.
+
+  A `remote` or `merged` side of another profile without `features.key_rotation` fails
+  the diff when `--filter` names `key_rotation`. Otherwise the diff shows
+  `--- key_rotation skipped: profile <name> has features.key_rotation disabled`
+  in place of the key rotation sections and still exits 0. A side of another profile
+  that enables the feature without the `manage_servers` scope always fails the diff.
+
+With the feature disabled, `--filter key_rotation` fails with
+`--filter key_rotation requires features.key_rotation: true`, and plain `pull`, `push`,
+and `diff` ignore key rotation.
 
 ## Templates
 

cmd/diff.go

@@ -1,13 +1,15 @@
 package cmd
 
 import (
+	"context"
+	"os"
+
 	"github.com/cloudentity/cac/internal/cac"
 	"github.com/cloudentity/cac/internal/cac/api"
 	"github.com/cloudentity/cac/internal/cac/diff"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"golang.org/x/exp/slog"
-	"os"
 )
 
 var (
@@ -41,9 +43,8 @@ Examples:
 		RunE: func(cmd *cobra.Command, args []string) error {
 			var (
 				app    *cac.Application
+				s      scope
 				result string
-				source api.Source
-				target api.Source
 				err    error
 			)
 
@@ -63,23 +64,11 @@ Examples:
 				return err
 			}
 
-			if source, err = app.PickSource(diffConfig.Source, rootConfig.Tenant); err != nil {
-				return err
-			}
-
-			if target, err = app.PickSource(diffConfig.Target, rootConfig.Tenant); err != nil {
+			if s, err = scopeOf(app.Config.KeyRotationEnabled(), diffConfig.Filters); err != nil {
 				return err
 			}
 
-			slog.Info("Comparing configurations", "source", source, "target", target)
-
-			if result, err = diff.Diff(cmd.Context(), source, target, rootConfig.Workspace,
-				diff.Colorize(diffConfig.Colors),
-				diff.OnlyPresent(diffConfig.OnlyPresent),
-				diff.Filters(diffConfig.Filters...),
-				diff.WithSecrets(diffConfig.WithSecrets),
-				diff.FilterVolatileFields(diffConfig.FilterVolatile),
-			); err != nil {
+			if result, err = runDiff(cmd.Context(), app, s, diffConfig.Source, diffConfig.Target); err != nil {
 				return err
 			}
 
@@ -98,6 +87,48 @@ Examples:
 	}
 )
 
+// runDiff compares the configuration and the key rotation of the source and target, as scoped.
+func runDiff(ctx context.Context, app *cac.Application, s scope, sourceSpec, targetSpec string) (string, error) {
+	var (
+		result string
+		kr     string
+		source api.Source
+		target api.Source
+		err    error
+	)
+
+	if s.regular {
+		if source, err = app.PickSource(sourceSpec, rootConfig.Tenant); err != nil {
+			return "", err
+		}
+
+		if target, err = app.PickSource(targetSpec, rootConfig.Tenant); err != nil {
+			return "", err
+		}
+
+		slog.Info("Comparing configurations", "source", source, "target", target)
+
+		if result, err = diff.Diff(ctx, source, target, rootConfig.Workspace,
+			diff.Colorize(diffConfig.Colors),
+			diff.OnlyPresent(diffConfig.OnlyPresent),
+			diff.Filters(diffConfig.Filters...),
+			diff.WithSecrets(diffConfig.WithSecrets),
+			diff.FilterVolatileFields(diffConfig.FilterVolatile),
+		); err != nil {
+			return "", err
+		}
+	}
+
+	if s.keyRotation {
+		if kr, err = diffKeyRotations(ctx, app, s, sourceSpec, targetSpec); err != nil {
+			return "", err
+		}
+	}
+
+	// a non-empty configuration diff ends with a newline, so the key rotation lines start their own
+	return result + kr, nil
+}
+
 func writeDiffResult(result string) error {
 	if diffConfig.Out != "-" {
 		if err := os.WriteFile(diffConfig.Out, []byte(result), 0644); err != nil {
@@ -139,6 +170,7 @@ Workspace resources: clients, idps, claims, custom_apps, gateways, policies, pol
                      ciba (alias of ciba_authentication_service)
 Tenant resources:    pools, schemas, mfa_methods, themes, servers
 Reserved:            root (only root-level tenant/workspace config, excluding nested resources)
+Key rotation:        key_rotation (workspace key rotation settings; requires features.key_rotation: true)
 Examples:
   --filter clients
   --filter clients,idps,policies