diff --git a/.sbomber-manifest-snap.yaml b/.github/.sbomber-manifest-snap.yaml similarity index 100% rename from .sbomber-manifest-snap.yaml rename to .github/.sbomber-manifest-snap.yaml diff --git a/.github/workflows/binaries.yml b/.github/workflows/binaries.yml index 76174617e..221c8b3be 100644 --- a/.github/workflows/binaries.yml +++ b/.github/workflows/binaries.yml @@ -20,7 +20,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v6 with: fetch-depth: 0 diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml index 3a5a0c7bc..22a3aee98 100644 --- a/.github/workflows/docs.yaml +++ b/.github/workflows/docs.yaml @@ -1,42 +1,39 @@ -# name: Docs on: push: branches: - - main + - master pull_request: - workflow_call: workflow_dispatch: -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true +permissions: {} jobs: - docchecks: - name: Checks - runs-on: ubuntu-22.04 - outputs: - linkcheck-result: ${{ steps.linkcheck-step.outcome }} + docs-spelling: + name: Spelling + runs-on: ubuntu-latest steps: - - name: Checkout - uses: actions/checkout@v4 + - uses: actions/checkout@v6 with: - fetch-depth: 0 - - name: Python + persist-credentials: false + - name: Set up Python uses: actions/setup-python@v6 with: python-version: "3.12" - - name: Links - id: linkcheck-step - if: success() || failure() - uses: canonical/documentation-workflows/linkcheck@main + - name: Check spelling + run: make -C docs spelling + + docs-linkcheck: + name: Links + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 with: - working-directory: docs - - name: Markdown lint - id: markdown-step - if: success() || failure() - uses: DavidAnson/markdownlint-cli2-action@v16 + persist-credentials: false + - name: Set up Python + uses: actions/setup-python@v6 with: - config: "docs/.sphinx/.markdownlint.json" + python-version: "3.12" + - name: Check links + run: make -C docs linkcheck diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 4ffc9406b..a686537eb 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -12,7 +12,7 @@ jobs: name: tests steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Set up Go uses: actions/setup-go@v6 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 94998be4f..119682c06 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -5,7 +5,7 @@ jobs: lint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v6 - uses: actions/setup-go@v6 with: @@ -32,7 +32,7 @@ jobs: staticcheck: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v6 - uses: actions/setup-go@v6 with: diff --git a/.github/workflows/sbom-secscan.yaml b/.github/workflows/sbom-secscan.yaml index d1f3900c0..4bf3b4305 100644 --- a/.github/workflows/sbom-secscan.yaml +++ b/.github/workflows/sbom-secscan.yaml @@ -11,12 +11,12 @@ jobs: scan: runs-on: [self-hosted, self-hosted-linux-amd64-jammy-private-endpoint-medium] steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 with: persist-credentials: false - name: sbomber uses: canonical/sbomber/.github/actions/run@cf7a76e810497ec932e8285b2c9d6b373d225e79 # main -- and sbomber-ref input should match with: - manifest: '${{ github.workspace }}/.sbomber-manifest-snap.yaml' + manifest: '${{ github.workspace }}/.github/.sbomber-manifest-snap.yaml' artifact-name: secscan-report-upload-snap sbomber-ref: cf7a76e810497ec932e8285b2c9d6b373d225e79 diff --git a/.github/workflows/scanning.yml b/.github/workflows/scanning.yml index b41bb6f48..b359ebf4f 100644 --- a/.github/workflows/scanning.yml +++ b/.github/workflows/scanning.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout repository - uses: actions/checkout@v2 + uses: actions/checkout@v6 - name: Run Github Trivy FS Action uses: aquasecurity/trivy-action@master diff --git a/.github/workflows/snap.yml b/.github/workflows/snap.yml index 4e13bdcea..cd8ae913a 100644 --- a/.github/workflows/snap.yml +++ b/.github/workflows/snap.yml @@ -26,7 +26,7 @@ jobs: if: github.event_name == 'pull_request' steps: - name: Checkout Pebble repo - uses: actions/checkout@v4 + uses: actions/checkout@v6 with: fetch-depth: 0 @@ -68,7 +68,7 @@ jobs: arch: ["amd64", "arm64", "armhf"] steps: - name: Checkout Pebble repo - uses: actions/checkout@v4 + uses: actions/checkout@v6 with: fetch-depth: 0 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 853e50de8..5e884755e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -12,7 +12,7 @@ jobs: name: Unit tests steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v6 - name: Set up Go uses: actions/setup-go@v6 @@ -29,7 +29,7 @@ jobs: name: Root Tests steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v6 - name: Set up Go uses: actions/setup-go@v6 @@ -45,7 +45,7 @@ jobs: name: Format check steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v6 - name: Set up Go uses: actions/setup-go@v6 @@ -66,7 +66,7 @@ jobs: name: Automated docs check steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v6 - name: Set up Python uses: actions/setup-python@v6 diff --git a/.github/workflows/tiobe.yaml b/.github/workflows/tiobe.yaml index 4493cf563..cef6b3c03 100644 --- a/.github/workflows/tiobe.yaml +++ b/.github/workflows/tiobe.yaml @@ -11,7 +11,7 @@ jobs: TICS: runs-on: [self-hosted, reactive, amd64, tiobe, noble] steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 with: persist-credentials: false @@ -36,7 +36,7 @@ jobs: gocov-xml < coverage.json > .coverage/coverage.xml - name: TICS GitHub Action - uses: tiobe/tics-github-action@009979693978bfefad2ad15c1020066694968dc7 # 3.4.0 + uses: tiobe/tics-github-action@768de18bedf164ee461bc9ef5e2f2fa1a20b122a # 3.7.0 with: mode: qserver viewerUrl: https://canonical.tiobe.com/tiobeweb/TICS/api/cfg?name=GoProjects diff --git a/HACKING.md b/HACKING.md index b9d3f0031..df47963fe 100644 --- a/HACKING.md +++ b/HACKING.md @@ -281,17 +281,27 @@ Recommended tone: ## Creating a release -To create a new tagged release, go to the [GitHub Releases page](https://github.com/canonical/pebble/releases) and: +When releasing, you need to release the `master` branch and the `fips` branch. -- Update `Version` in `cmd/version.go` to the version you're about to publish, for example `v1.9.0`, open a pull request, have it reviewed and merged into the `master` branch. -- Open a pull request to merge the `master` branch into the `fips` branch, resolve any conflicts, and have it reviewed and merged. -- Click ["Draft a new release"](https://github.com/canonical/pebble/releases/new). -- Enter the version tag (for example `v1.9.0`) and select "Create new tag: on publish". +Binaries will be created and uploaded automatically to this release by the [binaries.yml](https://github.com/canonical/pebble/blob/master/.github/workflows/binaries.yml) GitHub Action. In addition, a new Snap version is built and uploaded to the [Snap Store](https://snapcraft.io/pebble) (but promotion to `stable` is a manual step). + +Follow these steps: + +### Main release (master branch) + +- Update `Version` in `cmd/version.go` to the version you're about to publish, for example `v1.27.0`, open a pull request, have it reviewed and merged into the `master` branch. +- [Draft a new GitHub release](https://github.com/canonical/pebble/releases/new). +- Enter the version tag (for example `v1.27.0`) and select "Create new tag: on publish". - Enter a release title: include the version tag and a short summary of the release. - Write release notes: describe new features and bug fixes, and include a link to the full list of commits. - Click "Publish release". -- Likewise, publish a FIPS release (for example `v1.9.0-fips`) targeting the tip of the `fips` branch. -- Once the release GitHub Actions have finished, and the new [Snap](https://snapcraft.io/pebble) has been successfully built, update `Version` again to `v1.{next}.0-dev` (for example `v1.10.0-dev`). -- Find the security scan artifact on the 'SBOM and secscan' run corresponding the pushing the release tag, and upload it to the [SSDLC Pebble folder in Drive](https://drive.google.com/drive/folders/11WR629JFPJ8IMPI0kcsNp_qdf39c6no3). Open the artifact and verify that the security scan has not found any vulnerabilities. +- Monitor the release [GitHub Actions](https://github.com/canonical/pebble/actions) and check that the [snap](https://snapcraft.io/pebble) is uploaded correctly. +- If you're confident it's a compatible release, [promote the `candidate` snap to `stable`](https://snapcraft.io/pebble/releases). +- Find the security scan artifact on the corresponding [SBOM and secscan](https://github.com/canonical/pebble/actions/workflows/sbom-secscan.yaml) run, and upload it to the [SSDLC Pebble folder in Drive](https://drive.google.com/drive/folders/11WR629JFPJ8IMPI0kcsNp_qdf39c6no3). Open the artifact and verify that the security scan has not found any vulnerabilities. + +### FIPS release (fips branch) -Binaries will be created and uploaded automatically to this release by the [binaries.yml](https://github.com/canonical/pebble/blob/master/.github/workflows/binaries.yml) GitHub Actions job. In addition, a new Snap version is built and uploaded to the [Snap Store](https://snapcraft.io/pebble). +- Open a pull request to merge the `master` branch into the `fips` branch, resolve any conflicts, and have it reviewed and merged. +- Open a second PR on the `fips` branch to bump the `Version` in `cmd/version.go` to the FIPS version, for example `v1.27.0-fips`, and have it reviewed and merged. +- Publish a FIPS GitHub release (for example `v1.27.0-fips`) targeting the tip of the `fips` branch. +- As with the main release, monitor the release [GitHub Actions](https://github.com/canonical/pebble/actions), check that the FIPS snap is uploaded, and promote it to stable. diff --git a/docs/.custom_wordlist.txt b/docs/.custom_wordlist.txt index 734fdade6..4757c08db 100644 --- a/docs/.custom_wordlist.txt +++ b/docs/.custom_wordlist.txt @@ -1 +1,16 @@ # Leave a blank line at the end of this file to support concatenation +backoff +boolean +Drepper +idempotently +liveness +mkdir +ns +runit +replan +subcommands +supervisord +systemd +UIDs +Ulrich +Utils diff --git a/docs/.sphinx/.markdownlint.json b/docs/.sphinx/.markdownlint.json deleted file mode 100644 index 536f9ea92..000000000 --- a/docs/.sphinx/.markdownlint.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "default": false, - "MD003": { - "style": "atx" - }, - "MD014": true, - "MD018": true, - "MD022": true, - "MD023": true, - "MD026": { - "punctuation": ".,;。，；" - }, - "MD031": { - "list_items": false - }, - "MD032": true, - "MD035": true, - "MD042": true, - "MD045": true, - "MD052": true -} \ No newline at end of file diff --git a/docs/.sphinx/.pre-commit-config.yaml b/docs/.sphinx/.pre-commit-config.yaml deleted file mode 100644 index 07e0b48cb..000000000 --- a/docs/.sphinx/.pre-commit-config.yaml +++ /dev/null @@ -1,23 +0,0 @@ -repos: - - repo: local - hooks: - - id: make-spelling - name: Run make spelling - entry: make -C docs spelling - language: system - pass_filenames: false - files: ^docs/.*\.(rst|md|txt)$ - - - id: make-linkcheck - name: Run make linkcheck - entry: make -C docs linkcheck - language: system - pass_filenames: false - files: ^docs/.*\.(rst|md|txt)$ - - - id: make-woke - name: Run make woke - entry: make -C docs woke - language: system - pass_filenames: false - files: ^docs/.*\.(rst|md|txt)$ diff --git a/docs/.sphinx/.wordlist.txt b/docs/.sphinx/.wordlist.txt deleted file mode 100644 index 32bcd7c0b..000000000 --- a/docs/.sphinx/.wordlist.txt +++ /dev/null @@ -1,329 +0,0 @@ -ACME -ACME's -addons -AGPLv -API -APIs -balancer -Charmhub -CLI -DCO -Diátaxis -Dqlite -dropdown -EBS -EKS -enablement -favicon -Furo -Git -GitHub -Grafana -IAM -installable -JSON -Juju -Kubeflow -Kubernetes -Launchpad -linter -LTS -LXD -Makefile -Makefiles -Matrix -Mattermost -MicroCeph -MicroCloud -MicroOVN -MyST -namespace -namespaces -NodePort -Numbat -observability -OEM -OLM -Permalink -pre -Quickstart -ReadMe -reST -reStructuredText -roadmap -RTD -subdirectories -subfolders -subtree -TODO -Ubuntu -UI -UUID -VM -webhook -YAML - -AAM -AAR -AArch -ABI -ADB -ADSys -AKS -AMC -AMS -ANAIS -APK -APM -ARMv -ARR -ASIC -AZ -BAU -BOM -BTS -BZR -CAC -CBR -CDK -CDN -CISC -CLA -CMP -CNI -COS -COTS -CPC -CPQ -CPT -CR -CRD -CRI -CSB -CSI -CTR -CVE -DBR -DEP -DHCP -DKMS -DMB -DNS -DPU -DSA -DSE -EE -EFI -EoL -EoS -EPMO -ESM -FCE -FFE -FIPS -FOSS -FPGA -FTBFS -FTI -FUSE -GKE -GPGPU -GPL -GSI -GTM -HA -HAP -HPC -HWE -IC -IHV -IOV -IPMI -IPU -IRCC -iSCSI -ISM -ISR -ISV -ITP -JAAS -KSPP -LTV -MAAS -MDF -MDL -MEC -MLflow -MOTU -MPA -MQL -MRE -MSRP -NATS -NFS -NIC -NIH -NIST -NOC -NOS -NPOASR -NPV -NRE -NUMA -NVIU -NVMe -OBSD -OCI -OCL -OCTO -ODM -OSD -OSS -OVA -OVAL -OVN -OVS -P4 -PCB -PCF -PCKN -PCRE -PKCS -PKS -PMO -POSIX -PPA -PS5 -PXE -RACI -RADOS -RAG -RBAC -RDMA -RTOS -SDK -SDR -SDS -SEG -SFDC -SIP -SKU -SLA -SOW -SQL -SR -SRU -TAM -TCS -TFTP -TLS -TPM -TUI -UCA -UCT -UDS -UIFe -URI -UX -VCS -VNFD -WSL -ZFS - -Anbox -Ansible -Appium -Backports -Ceph -Changelog -Charmcraft -CoC -CoF -Containerd -Coturn -Cryptographic -Devel -Endian -Endianness -Fluentd -GraphQL -HAProxy -HAcluster -Imagecraft -Infiniband -Influxdb -Istio -Kata -Keepalived -Keyring -Keyserver -Kibana -Knative -LXC -Laravel -Libvirt -Linkerd -LinuxONE -Livepatch -Mesos -Metallb -MicroK -MicroStack -Mojo -Multipass -Nagios -NetApp -Netplan -Nginx -OpenSSL -OpenSearch -OpenStack -Openshift -PgBouncer -QEMU -RabbitMQ -Rasterization -Rebase -RegEx -RoM -RoP -RoQA -RoSRM -RoST -Rockcraft -SQLair -Scrcpy -Snapcraft -Snapd -Splunk -Subnet -Superdistro -Superset -Telegraf -Terraform -Testflinger -Thruk -Tigera -TrilioVault -TripleO -Vue -WebRTC -WoU -amd -armhf -autopkgtest -cURL -containeragent -dqlite -dsc -init -jQuery -juju -jujuc -jujud -ppc -riscv -snapcrafting -subcluster -swrast -zSystems diff --git a/docs/.sphinx/get_vale_conf.py b/docs/.sphinx/get_vale_conf.py index 46f57d894..13e7966f8 100644 --- a/docs/.sphinx/get_vale_conf.py +++ b/docs/.sphinx/get_vale_conf.py @@ -17,7 +17,7 @@ SPHINX_DIR = os.path.join(os.getcwd(), ".sphinx") -GITHUB_REPO = "canonical/praecepta" +GITHUB_REPO = "canonical/documentation-style-guide" GITHUB_CLONE_URL = f"https://github.com/{GITHUB_REPO}.git" # Source paths to copy from repo @@ -31,12 +31,12 @@ def clone_repo_and_copy_paths(file_source_dest, overwrite=False): """ Clone the repository to a temporary directory and copy required files - + Args: file_source_dest: dictionary of file paths to copy from the repository, and their destination paths overwrite: boolean flag to overwrite existing files in the destination - + Returns: bool: True if all files were copied successfully, False otherwise """ @@ -52,8 +52,8 @@ def clone_repo_and_copy_paths(file_source_dest, overwrite=False): try: result = subprocess.run( - clone_cmd, - capture_output=True, + clone_cmd, + capture_output=True, text=True, check=True ) @@ -73,7 +73,7 @@ def clone_repo_and_copy_paths(file_source_dest, overwrite=False): continue if not copy_files_to_path(source_path, dest, overwrite): - is_copy_success = False + is_copy_success = False logging.error("Failed to copy %s to %s", source_path, dest) # Clean up temporary directory @@ -85,12 +85,12 @@ def clone_repo_and_copy_paths(file_source_dest, overwrite=False): def copy_files_to_path(source_path, dest_path, overwrite=False): """ Copy a file or directory from source to destination - + Args: source_path: Path to the source file or directory dest_path: Path to the destination overwrite: Boolean flag to overwrite existing files in the destination - + Returns: bool: True if copy was successful, False otherwise """ @@ -138,7 +138,7 @@ def main(): # Parse command line arguments, default to overwrite_enabled = True overwrite_enabled = not parse_arguments().no_overwrite - # Download into /tmp through git clone + # Download into /tmp through git clone if not clone_repo_and_copy_paths(vale_files_dict, overwrite=overwrite_enabled): logging.error("Failed to download files from repository") return 1 diff --git a/docs/.sphinx/metrics/build_metrics.sh b/docs/.sphinx/metrics/build_metrics.sh deleted file mode 100755 index bd1ff1cb4..000000000 --- a/docs/.sphinx/metrics/build_metrics.sh +++ /dev/null @@ -1,15 +0,0 @@ -#!/bin/bash -# shellcheck disable=all - -links=0 -images=0 - -# count number of links -links=$(find . -type d -path './.sphinx' -prune -o -name '*.html' -exec cat {} + | grep -o " $(SPHINXDIR)/styles/woke.filter @echo '.Level=="error" and .Name!="Canonical.500-Repeated-words" and .Name!="Canonical.000-US-spellcheck"' > $(SPHINXDIR)/styles/error.filter @echo '.Name=="Canonical.000-US-spellcheck"' > $(SPHINXDIR)/styles/spelling.filter - @. $(VENV); find $(SPHINXDIR)/venv/lib/python*/site-packages/vale/vale_bin -size 195c -exec vale --version \; + @. $(VENV); find $(VALEDIR)/vale_bin -size 195c -exec vale --version \; woke: vale-install - @cat $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept.txt > $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept_backup.txt - @cat $(SOURCEDIR)/.custom_wordlist.txt >> $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept.txt + @cat $(VOCAB_CANONICAL)/accept.txt > $(VOCAB_CANONICAL)/accept_backup.txt + @cat $(SOURCEDIR)/.custom_wordlist.txt >> $(VOCAB_CANONICAL)/accept.txt @echo "Running Vale acceptable term check against $(TARGET). To change target set TARGET= with make command" @. $(VENV); vale --config="$(VALE_CONFIG)" --filter='$(SPHINXDIR)/styles/woke.filter' --glob='*.{md,rst}' $(TARGET) - @cat $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept_backup.txt > $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept.txt && rm $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept_backup.txt + @cat $(VOCAB_CANONICAL)/accept_backup.txt > $(VOCAB_CANONICAL)/accept.txt && rm $(VOCAB_CANONICAL)/accept_backup.txt vale: vale-install - @cat $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept.txt > $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept_backup.txt - @cat $(SOURCEDIR)/.custom_wordlist.txt >> $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept.txt + @cat $(VOCAB_CANONICAL)/accept.txt > $(VOCAB_CANONICAL)/accept_backup.txt + @cat $(SOURCEDIR)/.custom_wordlist.txt >> $(VOCAB_CANONICAL)/accept.txt @echo "Running Vale against $(TARGET). To change target set TARGET= with make command" @. $(VENV); vale --config="$(VALE_CONFIG)" --filter='$(SPHINXDIR)/styles/error.filter' --glob='*.{md,rst}' $(TARGET) - @cat $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept_backup.txt > $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept.txt && rm $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept_backup.txt + @cat $(VOCAB_CANONICAL)/accept_backup.txt > $(VOCAB_CANONICAL)/accept.txt && rm $(VOCAB_CANONICAL)/accept_backup.txt spelling: vale-install - @cat $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept.txt > $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept_backup.txt - @cat $(SOURCEDIR)/.custom_wordlist.txt >> $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept.txt + @cat $(VOCAB_CANONICAL)/accept.txt > $(VOCAB_CANONICAL)/accept_backup.txt + @cat $(SOURCEDIR)/.custom_wordlist.txt >> $(VOCAB_CANONICAL)/accept.txt @echo "Running Vale against $(TARGET). To change target set TARGET= with make command" @. $(VENV); vale --config="$(VALE_CONFIG)" --filter='$(SPHINXDIR)/styles/spelling.filter' --glob='*.{md,rst}' $(TARGET) - @cat $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept_backup.txt > $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept.txt && rm $(SPHINXDIR)/styles/config/vocabularies/Canonical/accept_backup.txt + @cat $(VOCAB_CANONICAL)/accept_backup.txt > $(VOCAB_CANONICAL)/accept.txt && rm $(VOCAB_CANONICAL)/accept_backup.txt spellcheck: spelling @echo "Please note that the \`make spellcheck\` command is being deprecated in favor of \`make spelling\`" @@ -169,21 +170,21 @@ allmetrics: html @echo "Recording documentation metrics..." @echo "Checking for existence of vale..." . $(VENV) - @. $(VENV); test -d $(SPHINXDIR)/venv/lib/python*/site-packages/vale || pip install vale + @. $(VENV); test -d $(VALEDIR) || pip install vale @. $(VENV); test -f $(VALE_CONFIG) || python3 $(SPHINXDIR)/get_vale_conf.py - @. $(VENV); find $(SPHINXDIR)/venv/lib/python*/site-packages/vale/vale_bin -size 195c -exec vale --config "$(VALE_CONFIG)" $(TARGET) > /dev/null \; + @. $(VENV); find $(VALEDIR)/vale_bin -size 195c -exec vale --config "$(VALE_CONFIG)" $(TARGET) > /dev/null \; @eval '$(METRICSDIR)/source_metrics.sh $(PWD)' - @$(METRICSDIR)/build_metrics.py $(BUILDDIR) + @. $(VENV); python3 $(METRICSDIR)/build_metrics.py $(BUILDDIR) update: install @. $(VENV); .sphinx/update_sp.py +cli-help: + @echo "Regenerating help output in CLI reference..." + @python3 generate_command_docs.py + # Catch-all target: route all unknown targets to Sphinx using the new # "make mode" option. $(O) is meant as a shortcut for $(SPHINXOPTS). %: $(MAKE) --no-print-directory install . $(VENV); $(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O) - -cli-help: - @echo "Regenerating help output in CLI reference..." - @python3 generate_command_docs.py diff --git a/docs/conf.py b/docs/conf.py index 1ae73dd53..b3950709e 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -64,13 +64,26 @@ copyright = "%s, %s" % (datetime.date.today().year, author) -# Documentation website URL and sitemap -# -# NOTE: 'html_baseurl' and 'sitemap_url_scheme' are used by the sphinx_sitemap -# extension. See https://sphinx-sitemap.readthedocs.io/ +# Use RTD canonical URL to ensure duplicate pages have a specific canonical URL + +html_baseurl = os.environ.get("READTHEDOCS_CANONICAL_URL", "/") + +# sphinx-sitemap uses html_baseurl to generate the full URL for each page: + +sitemap_url_scheme = '{link}' + +# Include `lastmod` dates in the sitemap: + +sitemap_show_lastmod = True + +# Exclude generated pages from the sitemap: + +sitemap_excludes = [ + '404/', + 'genindex/', + 'search/', +] -html_baseurl = "https://documentation.ubuntu.com/pebble/" -sitemap_url_scheme = "{link}" # TODO: Update with the official URL of your docs or leave empty if unsure. # @@ -91,7 +104,7 @@ # # TODO: To customise the preview image, update as needed. -ogp_image = "https://assets.ubuntu.com/v1/253da317-image-document-ubuntudocs.svg" +ogp_image = "https://assets.ubuntu.com/v1/cc828679-docs_illustration.svg" # Product favicon; shown in bookmarks, browser tabs, etc. diff --git a/docs/explanation/api-and-clients.md b/docs/explanation/api-and-clients.md index c15bdf3e3..5047fe072 100644 --- a/docs/explanation/api-and-clients.md +++ b/docs/explanation/api-and-clients.md @@ -1,6 +1,6 @@ # API and clients -The Pebble daemon exposes an API (HTTP over a unix socket) to allow remote clients to interact with the daemon. It can start and stop services, add configuration layers to the plan, and so on. +The Pebble daemon exposes an API (HTTP over a Unix socket) to allow remote clients to interact with the daemon. It can start and stop services, add configuration layers to the plan, and so on. If `pebble run` is started with the `--http
` option, Pebble also exposes open-access HTTP endpoints using the given TCP address (see {ref}`api-access-levels` below). @@ -52,7 +52,7 @@ Currently the supported authentication types are: - `local`: a local user ID determined using peer credentials. - `basic`: HTTP basic authentication. -An example admin identity named "bob" with `local` type is shown below: +An example admin identity named `bob` with `local` type is shown below: ```yaml identities: @@ -62,7 +62,7 @@ identities: user-id: 42 ``` -An example identity named "alice" with `basic` type and `metrics` access level is shown below: +An example identity named `alice` with `basic` type and `metrics` access level is shown below: ```yaml identities: diff --git a/docs/explanation/general-model.md b/docs/explanation/general-model.md index 3ad51342a..fbc8217d7 100644 --- a/docs/explanation/general-model.md +++ b/docs/explanation/general-model.md @@ -1,5 +1,5 @@ # General model -Pebble is organized as a single binary that works as a daemon and also as a client to itself. When the daemon runs it loads its own configuration from the `$PEBBLE` directory, as defined in the environment, and also records in that same directory its state and unix sockets for communication. If that variable is not defined, Pebble will attempt to look for its configuration from a default system-level setup at `/var/lib/pebble/default`. Using that directory is encouraged for whole-system setup such as when using Pebble to control services in a container. +Pebble is organized as a single binary that works as a daemon and also as a client to itself. When the daemon runs it loads its own configuration from the `$PEBBLE` directory, as defined in the environment, and also records in that same directory its state and Unix sockets for communication. If that variable is not defined, Pebble will attempt to look for its configuration from a default system-level setup at `/var/lib/pebble/default`. Using that directory is encouraged for whole-system setup such as when using Pebble to control services in a container. The `$PEBBLE` directory must contain a `layers/` subdirectory that holds a stack of configuration files with names similar to `001-base-layer.yaml`, where the digits define the order of the layer and the following label uniquely identifies it. Each layer in the stack sits above the former one, and has the chance to improve or redefine the service configuration as desired. diff --git a/docs/explanation/security.md b/docs/explanation/security.md index 4a47d84d2..a0a3bcd31 100644 --- a/docs/explanation/security.md +++ b/docs/explanation/security.md @@ -49,4 +49,4 @@ Our intention is that projects that build on Pebble can [override how TLS connec ### FIPS 140 -This project also distributes [FIPS 140](https://en.wikipedia.org/wiki/FIPS_140)-compliant builds of Pebble: the source code is in the `fips` branch and there's the `fips` track for the official [`pebble` snap](https://snapcraft.io/pebble). Refer to [HACKING.md](https://github.com/canonical/pebble/blob/fips/HACKING.md#fips-140-changes) in the `fips` branch for the list of limiations in the FIPS builds. +This project also distributes [FIPS 140](https://en.wikipedia.org/wiki/FIPS_140)-compliant builds of Pebble: the source code is in the `fips` branch and there's the `fips` track for the official [`pebble` snap](https://snapcraft.io/pebble). Refer to [HACKING.md](https://github.com/canonical/pebble/blob/fips/HACKING.md#fips-140-changes) in the `fips` branch for the list of limitations in the FIPS builds. diff --git a/docs/how-to/manage-identities.md b/docs/how-to/manage-identities.md index ef5d0bdb0..f6ba0414b 100644 --- a/docs/how-to/manage-identities.md +++ b/docs/how-to/manage-identities.md @@ -10,7 +10,7 @@ By default any UID (user ID) connected to the API socket is a `read` user; UID 0 ## Add new identities -To extend access to Pebble's API to additional users, add named identities using the [`add-identities`](#reference_pebble_add-identities_command) command with a YAML file configuring the details. For example, to add a new admin "bob" with UID 42 and a new read user "alice" with UID 2000, prepare this file: +To extend access to Pebble's API to additional users, add named identities using the [`add-identities`](#reference_pebble_add-identities_command) command with a YAML file configuring the details. For example, to add a new admin `bob` with UID 42 and a new read user `alice` with UID 2000, prepare this file: ```yaml # idents-add.yaml @@ -37,7 +37,7 @@ Added 2 new identities. ## Remove identities -To remove existing identities, use [`remove-identities`](#reference_pebble_remove-identities_command) with a YAML file that has a `null` value for each identity you want to remove. For example, to remove "alice", prepare this file: +To remove existing identities, use [`remove-identities`](#reference_pebble_remove-identities_command) with a YAML file that has a `null` value for each identity you want to remove. For example, to remove `alice`, prepare this file: ```yaml # idents-remove.yaml @@ -75,7 +75,7 @@ pebble update-identities --from idents-update.yaml Updated 1 identity. ``` -You can use the `--replace` flag to idempotently add or update (or even remove) identities, whether or not they exist. The replace option is useful in automated scripts. For example, to update "bob", add "alice", and remove "mallory" (if it exists), prepare this file: +You can use the `--replace` flag to idempotently add or update (or even remove) identities, whether or not they exist. The replace option is useful in automated scripts. For example, to update `bob`, add `alice`, and remove `mallory` (if it exists), prepare this file: ```yaml # idents-replace.yaml diff --git a/docs/index.md b/docs/index.md index 99c828ae2..62fc12e50 100644 --- a/docs/index.md +++ b/docs/index.md @@ -14,7 +14,7 @@ explanation/index It helps you orchestrate a set of local processes as an organized set. It resembles well-known tools such as _supervisord_, _runit_, or _s6_, in that it can easily manage non-system processes independently from the system services. However, it was designed with unique features such as layered configuration and an HTTP API that help with more specific use cases. -If you need a way to manage one or more services in a container, or as a non-root user on a machine, Pebble might be for you. It handles service logs, service dependencies, and allows you to set up ongoing health checks. Plus, it has an "HTTP over unix socket" API for all operations, with simple UID-based access control. +If you need a way to manage one or more services in a container, or as a non-root user on a machine, Pebble might be for you. It handles service logs, service dependencies, and allows you to set up ongoing health checks. Plus, it has an "HTTP over Unix socket" API for all operations, with simple UID-based access control. Pebble is useful for developers who are building {external+operator:ref}`Juju charms on Kubernetes `, creating {external+rockcraft:ref}`Rock ` or Docker images, or orchestrating services in the virtual machine. diff --git a/docs/reference/api.md b/docs/reference/api.md index 51a0b61f3..162f82515 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -77,7 +77,7 @@ Some `GET` requests take optional query parameters for configuring or filterin All data sent to the API in `POST` bodies and all response data from the API is in JSON format. Requests should have a `Content-Type: application/json` header. -There are two main types of requests: synchronous ("sync"), and asynchronous ("async") for operations that can take some time to execute. Synchronous responses have the following structure: +There are two main types of requests: synchronous (`sync`), and asynchronous (`async`) for operations that can take some time to execute. Synchronous responses have the following structure: ```json { diff --git a/docs/reference/cli-commands.md b/docs/reference/cli-commands.md index 9327a9a3f..14fc58f60 100644 --- a/docs/reference/cli-commands.md +++ b/docs/reference/cli-commands.md @@ -218,7 +218,7 @@ pebble exec pg_dump mydb ... ``` -The exec feature uses WebSockets under the hood, and allows you to stream stdin to the process, as well as stream stdout and stderr back. When running `pebble exec`, you can specify the working directory to run in (`-w`), environment variables to set (`--env`), and the user and group to run as (`--uid`/`--user` and `--gid`/`--group`). +The exec feature uses WebSockets under the hood, and allows you to stream `stdin` to the process, as well as stream `stdout` and `stderr` back. When running `pebble exec`, you can specify the working directory to run in (`-w`), environment variables to set (`--env`), and the user and group to run as (`--uid`/`--user` and `--gid`/`--group`). You can also apply a timeout with `--timeout`, for example: @@ -361,7 +361,7 @@ The identity command shows details for a single identity in YAML format. (reference_pebble_logs_command)= ## logs -The Pebble daemon's service manager stores the most recent stdout and stderr from each service, using a 100KB ring buffer per service. Each log line is prefixed with an RFC-3339 timestamp and the `[service-name]` in square brackets. +The Pebble daemon's service manager stores the most recent `stdout` and `stderr` from each service, using a 100KB ring buffer per service. Each log line is prefixed with an RFC-3339 timestamp and the `[service-name]` in square brackets. Logs are viewable via the logs API or using `pebble logs`: @@ -419,7 +419,7 @@ pebble logs --format=json {"time":"2022-11-14T01:39:13.889Z","service":"srv1","message":"Log 1 from srv1"} ``` -If you want to also write service logs to Pebble's own stdout, run the daemon with `--verbose`: +If you want to also write service logs to Pebble's own `stdout`, run the daemon with `--verbose`: ```{terminal} pebble run --verbose @@ -967,7 +967,7 @@ More ways to run the daemon: pebble run --args myservice --verbose --foo "multi str arg" ``` -* Use args terminator to pass `--hold` to Pebble at the end of the line: +* Use the argument terminator `;` to pass `--hold` to Pebble: ```bash pebble run --args myservice --verbose \; --hold diff --git a/docs/reference/environment-variables.md b/docs/reference/environment-variables.md index c6127a4b7..286a3eb7a 100644 --- a/docs/reference/environment-variables.md +++ b/docs/reference/environment-variables.md @@ -14,7 +14,7 @@ This will only copy the contents if the target directory, `$PEBBLE`, is empty. ## PEBBLE_DEBUG -If set to "1", debug logs will be printed to stderr. +If set to "1", debug logs will be printed to `stderr`. ## PEBBLE_PERSIST @@ -26,7 +26,7 @@ Pebble socket path. Defaults to `$PEBBLE/.pebble.socket` if not specified, or `/ ## PEBBLE_VERBOSE -If set to "1", the Pebble daemon writes service logs to stdout. +If set to "1", the Pebble daemon writes service logs to `stdout`. For `pebble run`, either `PEBBLE_VERBOSE=1` or the `--verbose` flag turns on verbose logging, with the command line flag overriding the environment variable. diff --git a/docs/reference/identities.md b/docs/reference/identities.md index 4b79bbeff..75bd17859 100644 --- a/docs/reference/identities.md +++ b/docs/reference/identities.md @@ -29,7 +29,7 @@ identities: password: ``` -For example, a local identity named "bob" with UID 42 that is granted `admin` access would be defined as follows: +For example, a local identity named `bob` with UID 42 that is granted `admin` access would be defined as follows: ```yaml identities: @@ -39,15 +39,14 @@ identities: user-id: 42 ``` -For another example, a basic identity named "alice" that is granted `metrics` access would be defined as follows: +For another example, a basic identity named `alice` that is granted `metrics` access would be defined as follows: ```yaml identities: alice: access: metrics basic: - # The password is hashed using sha512-crypt, as generated by "openssl passwd -6". password: ``` -The password is hashed using sha512-crypt, as generated by "openssl passwd -6". +The password is hashed using sha512-crypt, as generated by `openssl passwd -6`. diff --git a/docs/reference/index.md b/docs/reference/index.md index f4db96915..271d3a810 100644 --- a/docs/reference/index.md +++ b/docs/reference/index.md @@ -2,9 +2,9 @@ These guides provide technical information about Pebble. -% COMMENT: This toctree is for the navigation sidebar only -% Use an alphabetical listing of pages in the toctree -% For each page, make sure there's also a link in a section below +% COMMENT: This tree is for the navigation sidebar only. +% Use an alphabetical listing of pages in the tree. +% For each page, make sure there's also a link in a section below. ```{toctree} :hidden: @@ -24,7 +24,7 @@ Service auto-restart ``` -% COMMENT: The first few pages are presented in a more logical reading order +% COMMENT: The first few pages are presented in a more logical reading order. ## Layers diff --git a/docs/reference/log-forwarding.md b/docs/reference/log-forwarding.md index 8229f0404..11534cc49 100644 --- a/docs/reference/log-forwarding.md +++ b/docs/reference/log-forwarding.md @@ -22,7 +22,7 @@ Required configuration: - `override`: How this log target definition is combined with other pre-existing definitions with the same name in the plan. Supported values are `merge` and `replace`. - `type`: The type of log target. Supported types are `loki`, `opentelemetry` and `syslog`. -- `location`: The URL of the remote log target. For Loki, this needs to be the fully-qualified URL of the push API, including the API endpoint; use the format `http://:3100/loki/api/v1/push`. For OpenTelemetry, include the TCP port (normally 4318) without the API endpoint, for example: `http://:4318`. For Syslog, this needs to be either `tcp://:` or `udp://:`. +- `location`: The URL of the remote log target. For Loki, this needs to be the fully-qualified URL of the push API, including the API endpoint; use the format `http://:3100/loki/api/v1/push`. For OpenTelemetry, include the TCP port (normally 4318) without the API endpoint, for example: `http://:4318`. For `syslog`, this needs to be either `tcp://:` or `udp://:`. Optional configuration: diff --git a/internals/cli/cmd_checks.go b/internals/cli/cmd_checks.go index b19f26767..41995cd4d 100644 --- a/internals/cli/cmd_checks.go +++ b/internals/cli/cmd_checks.go @@ -34,6 +34,7 @@ arguments. type cmdChecks struct { client *client.Client + //lint:ignore SA5008 "choice" tag is intentionally duplicated Level string `long:"level" choice:"alive" choice:"ready"` Positional struct { Checks []string `positional-arg-name:""` diff --git a/internals/cli/cmd_health.go b/internals/cli/cmd_health.go index aa209a035..fab305a42 100644 --- a/internals/cli/cmd_health.go +++ b/internals/cli/cmd_health.go @@ -33,6 +33,7 @@ an exit code 1 if at least one of the requested checks are unhealthy. type cmdHealth struct { client *client.Client + //lint:ignore SA5008 "choice" tag is intentionally duplicated Level string `long:"level" choice:"alive" choice:"ready"` Positional struct { Checks []string `positional-arg-name:""` diff --git a/internals/cli/format.go b/internals/cli/format.go index 8d590fc18..f0ca60cb6 100644 --- a/internals/cli/format.go +++ b/internals/cli/format.go @@ -33,6 +33,7 @@ import ( ) type unicodeMixin struct { + //lint:ignore SA5008 "choice" tag is intentionally duplicated Unicode string `long:"unicode" default:"auto" choice:"auto" choice:"never" choice:"always"` } diff --git a/internals/daemon/access.go b/internals/daemon/access.go index 864176a45..4e0ec3c55 100644 --- a/internals/daemon/access.go +++ b/internals/daemon/access.go @@ -17,7 +17,7 @@ package daemon import ( "net/http" - "github.com/canonical/pebble/internals/overlord/state" + "github.com/canonical/pebble/internals/overlord/identities" ) const ( @@ -52,7 +52,7 @@ func (ac AdminAccess) CheckAccess(d *Daemon, r *http.Request, user *UserState) R // Not Unix Domain Socket or HTTPS. return Unauthorized(accessDenied) } - if user.Access == state.AdminAccess { + if user.Access == identities.AdminAccess { return nil } // An identity explicitly set to "access: read" or "access: untrusted" isn't allowed. @@ -73,7 +73,7 @@ func (ac UserAccess) CheckAccess(d *Daemon, r *http.Request, user *UserState) Re return Unauthorized(accessDenied) } switch user.Access { - case state.ReadAccess, state.AdminAccess: + case identities.ReadAccess, identities.AdminAccess: return nil } // An identity explicitly set to "access: untrusted" isn't allowed. @@ -95,7 +95,7 @@ func (ac MetricsAccess) CheckAccess(d *Daemon, r *http.Request, user *UserState) // HTTP access (only basic auth is possible here, so no need to // check with identity type). transport := RequestTransportType(r) - if transport == TransportTypeHTTP && user.Access == state.MetricsAccess { + if transport == TransportTypeHTTP && user.Access == identities.MetricsAccess { return nil } if !transport.IsConcealed() { @@ -103,7 +103,7 @@ func (ac MetricsAccess) CheckAccess(d *Daemon, r *http.Request, user *UserState) return Unauthorized(accessDenied) } switch user.Access { - case state.MetricsAccess, state.ReadAccess, state.AdminAccess: + case identities.MetricsAccess, identities.ReadAccess, identities.AdminAccess: return nil default: // All other access levels, including "access: untrusted", are denied. diff --git a/internals/daemon/access_test.go b/internals/daemon/access_test.go index 674b5d314..b2b1fa19b 100644 --- a/internals/daemon/access_test.go +++ b/internals/daemon/access_test.go @@ -22,7 +22,7 @@ import ( . "gopkg.in/check.v1" "github.com/canonical/pebble/internals/daemon" - "github.com/canonical/pebble/internals/overlord/state" + "github.com/canonical/pebble/internals/overlord/identities" ) type accessSuite struct{} @@ -52,7 +52,7 @@ func (s *accessSuite) TestAccess(c *C) { }, { // User access: UntrustedAccess apiSource: daemon.TransportTypeUnixSocket, - user: &daemon.UserState{Access: state.UntrustedAccess}, + user: &daemon.UserState{Access: identities.UntrustedAccess}, openCheckErr: nil, adminCheckErr: errUnauthorized, userCheckErr: errUnauthorized, @@ -60,7 +60,7 @@ func (s *accessSuite) TestAccess(c *C) { }, { // User access: MetricsAccess apiSource: daemon.TransportTypeUnixSocket, - user: &daemon.UserState{Access: state.MetricsAccess}, + user: &daemon.UserState{Access: identities.MetricsAccess}, openCheckErr: nil, adminCheckErr: errUnauthorized, userCheckErr: errUnauthorized, @@ -68,7 +68,7 @@ func (s *accessSuite) TestAccess(c *C) { }, { // User access: ReadAccess apiSource: daemon.TransportTypeUnixSocket, - user: &daemon.UserState{Access: state.ReadAccess}, + user: &daemon.UserState{Access: identities.ReadAccess}, openCheckErr: nil, adminCheckErr: errUnauthorized, userCheckErr: nil, @@ -76,7 +76,7 @@ func (s *accessSuite) TestAccess(c *C) { }, { // User access: AdminAccess apiSource: daemon.TransportTypeUnixSocket, - user: &daemon.UserState{Access: state.AdminAccess}, + user: &daemon.UserState{Access: identities.AdminAccess}, openCheckErr: nil, adminCheckErr: nil, userCheckErr: nil, @@ -94,7 +94,7 @@ func (s *accessSuite) TestAccess(c *C) { }, { // User access: UntrustedAccess apiSource: daemon.TransportTypeHTTP, - user: &daemon.UserState{Access: state.UntrustedAccess}, + user: &daemon.UserState{Access: identities.UntrustedAccess}, openCheckErr: nil, adminCheckErr: errUnauthorized, userCheckErr: errUnauthorized, @@ -102,7 +102,7 @@ func (s *accessSuite) TestAccess(c *C) { }, { // User access: MetricsAccess apiSource: daemon.TransportTypeHTTP, - user: &daemon.UserState{Access: state.MetricsAccess}, + user: &daemon.UserState{Access: identities.MetricsAccess}, openCheckErr: nil, adminCheckErr: errUnauthorized, userCheckErr: errUnauthorized, @@ -110,7 +110,7 @@ func (s *accessSuite) TestAccess(c *C) { }, { // User access: ReadAccess apiSource: daemon.TransportTypeHTTP, - user: &daemon.UserState{Access: state.ReadAccess}, + user: &daemon.UserState{Access: identities.ReadAccess}, openCheckErr: nil, adminCheckErr: errUnauthorized, userCheckErr: errUnauthorized, @@ -118,7 +118,7 @@ func (s *accessSuite) TestAccess(c *C) { }, { // User access: AdminAccess apiSource: daemon.TransportTypeHTTP, - user: &daemon.UserState{Access: state.AdminAccess}, + user: &daemon.UserState{Access: identities.AdminAccess}, openCheckErr: nil, adminCheckErr: errUnauthorized, userCheckErr: errUnauthorized, diff --git a/internals/daemon/api_identities.go b/internals/daemon/api_identities.go index 36feeced9..cebdeb6e9 100644 --- a/internals/daemon/api_identities.go +++ b/internals/daemon/api_identities.go @@ -16,76 +16,145 @@ package daemon import ( "encoding/json" + "errors" "fmt" "net/http" "github.com/canonical/pebble/internals/logger" - "github.com/canonical/pebble/internals/overlord/state" + "github.com/canonical/pebble/internals/overlord/identities" ) +// apiIdentity exists so the API marshalling of an Identity excludes secrets. +type apiIdentity struct { + Access identities.Access `json:"access"` + Local *apiLocalIdentity `json:"local,omitempty"` + Basic *apiBasicIdentity `json:"basic,omitempty"` + Cert *apiCertIdentity `json:"cert,omitempty"` +} + +type apiLocalIdentity struct { + // Needs to be a pointer so we can distinguish between missing and zero (UID 0). + UserID *uint32 `json:"user-id"` +} + +type apiBasicIdentity struct { + Password string `json:"password"` +} + +type apiCertIdentity struct { + PEM string `json:"pem"` +} + +// When adding a new identity type, be sure to mask secrets here. +func identityToAPI(d *identities.Identity) *apiIdentity { + ai := &apiIdentity{ + Access: d.Access, + } + if d.Local != nil { + ai.Local = &apiLocalIdentity{UserID: &d.Local.UserID} + } + if d.Basic != nil { + ai.Basic = &apiBasicIdentity{Password: "*****"} + } + return ai +} + +func identityFromAPI(ai *apiIdentity, name string) (*identities.Identity, error) { + if ai == nil { + return nil, nil + } + + identity := &identities.Identity{ + Access: ai.Access, + } + + if ai.Local != nil { + if ai.Local.UserID == nil { + return nil, errors.New("local identity must specify user-id") + } + identity.Local = &identities.LocalIdentity{UserID: *ai.Local.UserID} + } + if ai.Basic != nil { + identity.Basic = &identities.BasicIdentity{Password: ai.Basic.Password} + } + + // Perform additional validation using the local Identity type. + err := identity.Validate(name) + if err != nil { + return nil, err + } + + return identity, nil +} + func v1GetIdentities(c *Command, r *http.Request, _ *UserState) Response { st := c.d.overlord.State() st.Lock() defer st.Unlock() - identities := st.Identities() - return SyncResponse(identities) + identitiesMgr := c.d.overlord.IdentitiesManager() + idents := identitiesMgr.Identities() + + apiIdentities := make(map[string]*apiIdentity, len(idents)) + for name, identity := range idents { + apiIdentities[name] = identityToAPI(identity) + } + return SyncResponse(apiIdentities) } func v1PostIdentities(c *Command, r *http.Request, user *UserState) Response { var payload struct { - Action string `json:"action"` - Identities map[string]*state.Identity `json:"identities"` + Action string `json:"action"` + Identities map[string]*apiIdentity `json:"identities"` } decoder := json.NewDecoder(r.Body) if err := decoder.Decode(&payload); err != nil { return BadRequest("cannot decode request body: %v", err) } - var identityNames map[string]struct{} - switch payload.Action { - case "add", "update": - for name, identity := range payload.Identities { - if identity == nil { - return BadRequest(`identity value for %q must not be null for %s operation`, name, payload.Action) - } + idents := make(map[string]*identities.Identity, len(payload.Identities)) + for name, apiIdent := range payload.Identities { + identity, err := identityFromAPI(apiIdent, name) + if err != nil { + return BadRequest("invalid identity for %q: %v", name, err) } - case "replace": - break - case "remove": - identityNames = make(map[string]struct{}) - for name, identity := range payload.Identities { - if identity != nil { - return BadRequest(`identity value for %q must be null for %s operation`, name, payload.Action) - } - identityNames[name] = struct{}{} - } - default: - return BadRequest(`invalid action %q, must be "add", "update", "replace", or "remove"`, payload.Action) + idents[name] = identity } st := c.d.overlord.State() st.Lock() defer st.Unlock() + identitiesMgr := c.d.overlord.IdentitiesManager() + var err error switch payload.Action { case "add": - for name, identity := range payload.Identities { + for name, identity := range idents { + if identity == nil { + return BadRequest(`identity value for %q must not be null for add operation`, name) + } + } + for name, identity := range idents { logger.SecurityWarn(logger.SecurityUserCreated, fmt.Sprintf("%s,%s,%s", userString(user), name, identity.Access), fmt.Sprintf("Creating %s user %s", identity.Access, name)) } - err = st.AddIdentities(payload.Identities) + err = identitiesMgr.AddIdentities(idents) case "update": - for name, identity := range payload.Identities { + for name, identity := range idents { + if identity == nil { + return BadRequest(`identity value for %q must not be null for update operation`, name) + } + } + for name, identity := range idents { logger.SecurityWarn(logger.SecurityUserUpdated, fmt.Sprintf("%s,%s,%s", userString(user), name, identity.Access), fmt.Sprintf("Updating %s user %s", identity.Access, name)) } - err = st.UpdateIdentities(payload.Identities) + err = identitiesMgr.UpdateIdentities(idents) case "replace": - for name, identity := range payload.Identities { + for name, identity := range idents { if identity == nil { logger.SecurityWarn(logger.SecurityUserDeleted, fmt.Sprintf("%s,%s", userString(user), name), @@ -96,14 +165,23 @@ func v1PostIdentities(c *Command, r *http.Request, user *UserState) Response { fmt.Sprintf("Updating %s user %s", identity.Access, name)) } } - err = st.ReplaceIdentities(payload.Identities) + err = identitiesMgr.ReplaceIdentities(idents) case "remove": - for name := range payload.Identities { + identitiesToRemove := make(map[string]struct{}, len(idents)) + for name, identity := range idents { + if identity != nil { + return BadRequest(`identity value for %q must be null for remove operation`, name) + } + identitiesToRemove[name] = struct{}{} + } + for name := range identitiesToRemove { logger.SecurityWarn(logger.SecurityUserDeleted, fmt.Sprintf("%s,%s", userString(user), name), fmt.Sprintf("Deleting user %s", name)) } - err = st.RemoveIdentities(identityNames) + err = identitiesMgr.RemoveIdentities(identitiesToRemove) + default: + return BadRequest(`invalid action %q, must be "add", "update", "replace", or "remove"`, payload.Action) } if err != nil { return BadRequest("%v", err) diff --git a/internals/daemon/api_identities_test.go b/internals/daemon/api_identities_test.go index 8942b2d40..e06c4841c 100644 --- a/internals/daemon/api_identities_test.go +++ b/internals/daemon/api_identities_test.go @@ -16,13 +16,14 @@ package daemon import ( "encoding/json" + "fmt" "net/http" "strings" . "gopkg.in/check.v1" "github.com/canonical/pebble/internals/logger" - "github.com/canonical/pebble/internals/overlord/state" + "github.com/canonical/pebble/internals/overlord/identities" ) func (s *apiSuite) TestIdentities(c *C) { @@ -30,14 +31,15 @@ func (s *apiSuite) TestIdentities(c *C) { st := s.d.overlord.State() st.Lock() - err := st.AddIdentities(map[string]*state.Identity{ + identitiesMgr := s.d.overlord.IdentitiesManager() + err := identitiesMgr.AddIdentities(map[string]*identities.Identity{ "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, }, "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, }, }) c.Assert(err, IsNil) @@ -51,7 +53,7 @@ func (s *apiSuite) TestIdentities(c *C) { c.Check(rsp.Type, Equals, ResponseTypeSync) c.Check(rsp.Status, Equals, http.StatusOK) - identities, ok := rsp.Result.(map[string]*state.Identity) + identities, ok := rsp.Result.(map[string]*apiIdentity) c.Assert(ok, Equals, true) data, err := json.MarshalIndent(identities, "", " ") @@ -103,17 +105,18 @@ func (s *apiSuite) TestAddIdentities(c *C) { st := s.d.overlord.State() st.Lock() - identities := st.Identities() - c.Assert(identities, DeepEquals, map[string]*state.Identity{ + identitiesMgr := s.d.overlord.IdentitiesManager() + idents := identitiesMgr.Identities() + c.Assert(idents, DeepEquals, map[string]*identities.Identity{ "bob": { Name: "bob", - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, }, "mary": { Name: "mary", - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, }, }) st.Unlock() @@ -148,14 +151,15 @@ func (s *apiSuite) TestUpdateIdentities(c *C) { st := s.d.overlord.State() st.Lock() - err := st.AddIdentities(map[string]*state.Identity{ + identitiesMgr := s.d.overlord.IdentitiesManager() + err := identitiesMgr.AddIdentities(map[string]*identities.Identity{ "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, }, "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, }, }) c.Assert(err, IsNil) @@ -184,17 +188,17 @@ func (s *apiSuite) TestUpdateIdentities(c *C) { c.Check(rsp.Status, Equals, http.StatusOK) st.Lock() - identities := st.Identities() - c.Assert(identities, DeepEquals, map[string]*state.Identity{ + idents := identitiesMgr.Identities() + c.Assert(idents, DeepEquals, map[string]*identities.Identity{ "bob": { Name: "bob", - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, }, "mary": { Name: "mary", - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, }, }) st.Unlock() @@ -229,14 +233,15 @@ func (s *apiSuite) TestReplaceIdentities(c *C) { st := s.d.overlord.State() st.Lock() - err := st.AddIdentities(map[string]*state.Identity{ + identitiesMgr := s.d.overlord.IdentitiesManager() + err := identitiesMgr.AddIdentities(map[string]*identities.Identity{ "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, }, "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, }, }) c.Assert(err, IsNil) @@ -266,17 +271,17 @@ func (s *apiSuite) TestReplaceIdentities(c *C) { c.Check(rsp.Status, Equals, http.StatusOK) st.Lock() - identities := st.Identities() - c.Assert(identities, DeepEquals, map[string]*state.Identity{ + idents := identitiesMgr.Identities() + c.Assert(idents, DeepEquals, map[string]*identities.Identity{ "mary": { Name: "mary", - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 43}, + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 43}, }, "newguy": { Name: "newguy", - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 44}, + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 44}, }, }) st.Unlock() @@ -294,14 +299,15 @@ func (s *apiSuite) TestRemoveIdentities(c *C) { st := s.d.overlord.State() st.Lock() - err := st.AddIdentities(map[string]*state.Identity{ + identitiesMgr := s.d.overlord.IdentitiesManager() + err := identitiesMgr.AddIdentities(map[string]*identities.Identity{ "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, }, "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, }, }) c.Assert(err, IsNil) @@ -319,12 +325,12 @@ func (s *apiSuite) TestRemoveIdentities(c *C) { c.Check(rsp.Status, Equals, http.StatusOK) st.Lock() - identities := st.Identities() - c.Assert(identities, DeepEquals, map[string]*state.Identity{ + idents := identitiesMgr.Identities() + c.Assert(idents, DeepEquals, map[string]*identities.Identity{ "mary": { Name: "mary", - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, }, }) st.Unlock() @@ -371,6 +377,42 @@ func (s *apiSuite) TestPostIdentitiesInvalidAction(c *C) { c.Assert(result.Message, Matches, `invalid action "foobar", must be "add", "update", "replace", or "remove"`) } +func (s *apiSuite) TestUnmarshalErrors(c *C) { + s.daemon(c) + + tests := []struct { + data string + error string + }{{ + data: `{"no-type": {"access": "admin"}}`, + error: `identity must have at least one type \("local", "basic", or "cert"\)`, + }, { + data: `{"invalid-access": {"access": "admin", "local": {}}}`, + error: `local identity must specify user-id`, + }, { + data: `{"invalid-access": {"access": "metrics", "basic": {}}}`, + error: `basic identity must specify password \(hashed\)`, + }, { + data: `{"invalid-access": {"access": "foo", "local": {"user-id": 42}}}`, + error: `invalid access value "foo", must be "admin", "read", "metrics", or "untrusted"`, + }, { + data: `{"invalid-access": {"local": {"user-id": 42}}}`, + error: `access value must be specified \("admin", "read", "metrics", or "untrusted"\)`, + }} + for _, test := range tests { + c.Logf("Input data: %s", test.data) + + body := fmt.Sprintf(`{"action": "foobar", "identities": %s}`, test.data) + + rsp := s.postIdentities(c, body) + c.Check(rsp.Type, Equals, ResponseTypeError) + c.Check(rsp.Status, Equals, http.StatusBadRequest) + result, ok := rsp.Result.(*errorResult) + c.Assert(ok, Equals, true) + c.Check(result.Message, Matches, ".*: "+test.error) + } +} + func (s *apiSuite) postIdentities(c *C, body string) *resp { req, err := http.NewRequest("POST", "/v1/identities", strings.NewReader(body)) c.Assert(err, IsNil) diff --git a/internals/daemon/api_notices.go b/internals/daemon/api_notices.go index 8cd274508..2a4ccb496 100644 --- a/internals/daemon/api_notices.go +++ b/internals/daemon/api_notices.go @@ -26,6 +26,7 @@ import ( "github.com/canonical/x-go/strutil" + "github.com/canonical/pebble/internals/overlord/identities" "github.com/canonical/pebble/internals/overlord/state" ) @@ -54,7 +55,7 @@ func v1GetNotices(c *Command, r *http.Request, user *UserState) Response { query := r.URL.Query() if len(query["user-id"]) > 0 { - if user.Access != state.AdminAccess { + if user.Access != identities.AdminAccess { return Forbidden(`only admins may use the "user-id" filter`) } var err error @@ -65,7 +66,7 @@ func v1GetNotices(c *Command, r *http.Request, user *UserState) Response { } if len(query["users"]) > 0 { - if user.Access != state.AdminAccess { + if user.Access != identities.AdminAccess { return Forbidden(`only admins may use the "users" filter`) } if len(query["user-id"]) > 0 { @@ -263,7 +264,7 @@ func noticeViewableByUser(notice *state.Notice, user *UserState) bool { // Notice has no UID, so it's viewable by any user (with a UID). return true } - if user.Access == state.AdminAccess { + if user.Access == identities.AdminAccess { // User is admin, they can view anything. return true } diff --git a/internals/daemon/api_notices_test.go b/internals/daemon/api_notices_test.go index 607e9f869..58162f5e6 100644 --- a/internals/daemon/api_notices_test.go +++ b/internals/daemon/api_notices_test.go @@ -27,6 +27,7 @@ import ( . "gopkg.in/check.v1" + "github.com/canonical/pebble/internals/overlord/identities" "github.com/canonical/pebble/internals/overlord/state" ) @@ -86,7 +87,7 @@ func (s *apiSuite) testNoticesFilter(c *C, makeQuery func(after time.Time) url.V req, err := http.NewRequest("GET", "/v1/notices?"+query.Encode(), nil) c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices") - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.AdminAccess, 0)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.AdminAccess, 0)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -135,7 +136,7 @@ func (s *apiSuite) TestNoticesFilterMultipleTypes(c *C) { req, err := http.NewRequest("GET", "/v1/notices?types=change-update&types=warning,warning", nil) c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices") - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -164,7 +165,7 @@ func (s *apiSuite) TestNoticesFilterMultipleKeys(c *C) { req, err := http.NewRequest("GET", "/v1/notices?keys=a.b/x&keys=danger", nil) c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices") - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -192,7 +193,7 @@ func (s *apiSuite) TestNoticesFilterInvalidTypes(c *C) { req, err := http.NewRequest("GET", "/v1/notices?types=foo&types=warning&types=bar,baz", nil) c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices") - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -208,7 +209,7 @@ func (s *apiSuite) TestNoticesFilterInvalidTypes(c *C) { req, err = http.NewRequest("GET", "/v1/notices?types=foo&types=bar,baz", nil) c.Assert(err, IsNil) noticesCmd = apiCmd("/v1/notices") - rsp, ok = noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok = noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -240,7 +241,7 @@ func (s *apiSuite) TestNoticesUserIDAdminDefault(c *C) { // Test that admin user sees their own and all public notices if no filter is specified req, err := http.NewRequest("GET", "/v1/notices", nil) c.Assert(err, IsNil) - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.AdminAccess, 0)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.AdminAccess, 0)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -282,7 +283,7 @@ func (s *apiSuite) TestNoticesUserIDAdminFilter(c *C) { reqUrl := fmt.Sprintf("/v1/notices?%s", userIDValues.Encode()) req, err := http.NewRequest("GET", reqUrl, nil) c.Assert(err, IsNil) - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.AdminAccess, 0)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.AdminAccess, 0)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -319,7 +320,7 @@ func (s *apiSuite) TestNoticesUserIDNonAdminDefault(c *C) { // Test that non-admin user by default only sees their notices and public notices. req, err := http.NewRequest("GET", "/v1/notices", nil) c.Assert(err, IsNil) - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -350,7 +351,7 @@ func (s *apiSuite) TestNoticesUserIDNonAdminFilter(c *C) { reqUrl := "/v1/notices?user-id=1000" req, err := http.NewRequest("GET", reqUrl, nil) c.Assert(err, IsNil) - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeError) @@ -382,7 +383,7 @@ func (s *apiSuite) TestNoticesUsersAdminFilter(c *C) { reqUrl := "/v1/notices?users=all" req, err := http.NewRequest("GET", reqUrl, nil) c.Check(err, IsNil) - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.AdminAccess, 0)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.AdminAccess, 0)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -419,7 +420,7 @@ func (s *apiSuite) TestNoticesUsersNonAdminFilter(c *C) { reqUrl := "/v2/notices?users=all" req, err := http.NewRequest("GET", reqUrl, nil) c.Check(err, IsNil) - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeError) @@ -441,7 +442,7 @@ func (s *apiSuite) TestNoticesUnknownRequestUID(c *C) { // Test that a connection with unknown UID is forbidden from receiving notices req, err := http.NewRequest("GET", "/v1/notices", nil) c.Assert(err, IsNil) - rsp, ok := noticesCmd.GET(noticesCmd, req, &UserState{Access: state.ReadAccess}).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, &UserState{Access: identities.ReadAccess}).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeError) @@ -464,7 +465,7 @@ func (s *apiSuite) TestNoticesWait(c *C) { req, err := http.NewRequest("GET", "/v1/notices?timeout=1s", nil) c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices") - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -484,7 +485,7 @@ func (s *apiSuite) TestNoticesTimeout(c *C) { req, err := http.NewRequest("GET", "/v1/notices?timeout=1ms", nil) c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices") - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -508,7 +509,7 @@ func (s *apiSuite) TestNoticesRequestCancelled(c *C) { req, err := http.NewRequestWithContext(ctx, "GET", "/v1/notices?timeout=1s", nil) c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices") - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeError) @@ -560,7 +561,7 @@ func (s *apiSuite) testNoticesBadRequest(c *C, query, errorMatch string) { req, err := http.NewRequest("GET", "/v1/notices?"+query, nil) c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices") - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.AdminAccess, 0)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.AdminAccess, 0)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeError) @@ -584,7 +585,7 @@ func (s *apiSuite) TestAddNotice(c *C) { req, err := http.NewRequest("POST", "/v1/notices", bytes.NewReader(body)) c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices") - rsp, ok := noticesCmd.POST(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.POST(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -638,7 +639,7 @@ func (s *apiSuite) TestAddNoticeMinimal(c *C) { req, err := http.NewRequest("POST", "/v1/notices", bytes.NewReader(body)) c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices") - rsp, ok := noticesCmd.POST(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.POST(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -680,7 +681,7 @@ func (s *apiSuite) TestAddNoticeInvalidRequestUid(c *C) { req, err := http.NewRequest("POST", "/v1/notices", bytes.NewReader(body)) c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices") - rsp, ok := noticesCmd.POST(noticesCmd, req, &UserState{Access: state.ReadAccess}).(*resp) + rsp, ok := noticesCmd.POST(noticesCmd, req, &UserState{Access: identities.ReadAccess}).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeError) @@ -737,7 +738,7 @@ func (s *apiSuite) testAddNoticeBadRequest(c *C, body, errorMatch string) { req, err := http.NewRequest("POST", "/v1/notices", strings.NewReader(body)) c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices") - rsp, ok := noticesCmd.POST(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.POST(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeError) @@ -765,7 +766,7 @@ func (s *apiSuite) TestNotice(c *C) { c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices/{id}") s.vars = map[string]string{"id": noticeIDPublic} - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -781,7 +782,7 @@ func (s *apiSuite) TestNotice(c *C) { c.Assert(err, IsNil) noticesCmd = apiCmd("/v1/notices/{id}") s.vars = map[string]string{"id": noticeIDPrivate} - rsp, ok = noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok = noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -801,7 +802,7 @@ func (s *apiSuite) TestNoticeNotFound(c *C) { c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices/{id}") s.vars = map[string]string{"id": "1234"} - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1000)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1000)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeError) @@ -817,7 +818,7 @@ func (s *apiSuite) TestNoticeUnknownRequestUID(c *C) { c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices/{id}") s.vars = map[string]string{"id": "1234"} - rsp, ok := noticesCmd.GET(noticesCmd, req, &UserState{Access: state.ReadAccess}).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, &UserState{Access: identities.ReadAccess}).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeError) @@ -840,7 +841,7 @@ func (s *apiSuite) TestNoticeAdminAllowed(c *C) { c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices/{id}") s.vars = map[string]string{"id": noticeID} - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.AdminAccess, 0)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.AdminAccess, 0)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeSync) @@ -867,7 +868,7 @@ func (s *apiSuite) TestNoticeNonAdminNotAllowed(c *C) { c.Assert(err, IsNil) noticesCmd := apiCmd("/v1/notices/{id}") s.vars = map[string]string{"id": noticeID} - rsp, ok := noticesCmd.GET(noticesCmd, req, userState(state.ReadAccess, 1001)).(*resp) + rsp, ok := noticesCmd.GET(noticesCmd, req, userState(identities.ReadAccess, 1001)).(*resp) c.Assert(ok, Equals, true) c.Check(rsp.Type, Equals, ResponseTypeError) @@ -890,6 +891,6 @@ func addNotice(c *C, st *state.State, userID *uint32, noticeType state.NoticeTyp c.Assert(err, IsNil) } -func userState(access state.IdentityAccess, uid uint32) *UserState { +func userState(access identities.Access, uid uint32) *UserState { return &UserState{Access: access, UID: &uid} } diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go index ed1ad2777..ddafa75c1 100644 --- a/internals/daemon/daemon.go +++ b/internals/daemon/daemon.go @@ -40,6 +40,7 @@ import ( "github.com/canonical/pebble/internals/osutil" "github.com/canonical/pebble/internals/overlord" "github.com/canonical/pebble/internals/overlord/checkstate" + "github.com/canonical/pebble/internals/overlord/identities" "github.com/canonical/pebble/internals/overlord/restart" "github.com/canonical/pebble/internals/overlord/servstate" "github.com/canonical/pebble/internals/overlord/standby" @@ -172,7 +173,7 @@ type Daemon struct { // UserState represents the state of an authenticated API user. type UserState struct { - Access state.IdentityAccess + Access identities.Access UID *uint32 Username string } @@ -196,7 +197,7 @@ type Command struct { d *Daemon } -func userFromRequest(st *state.State, r *http.Request, ucred *Ucrednet) *UserState { +func userFromRequest(st *state.State, identitiesMgr *identities.Manager, r *http.Request, ucred *Ucrednet) *UserState { // Does the HTTP header include basic auth credentials? username, password, _ := r.BasicAuth() @@ -207,7 +208,7 @@ func userFromRequest(st *state.State, r *http.Request, ucred *Ucrednet) *UserSta } st.Lock() - identity := st.IdentityFromInputs(userID, username, password) + identity := identitiesMgr.IdentityFromInputs(userID, username, password) st.Unlock() if identity != nil { @@ -286,17 +287,18 @@ func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) { // not good: https://github.com/canonical/pebble/pull/369 var user *UserState if _, isOpen := access.(OpenAccess); !isOpen { - user = userFromRequest(c.d.state, r, ucred) + identitiesMgr := c.d.Overlord().IdentitiesManager() + user = userFromRequest(c.d.state, identitiesMgr, r, ucred) } // If we don't have a named-identity user, use ucred UID to see if we have a default. if user == nil && ucred != nil { if ucred.Uid == 0 || ucred.Uid == uint32(os.Getuid()) { // Admin if UID is 0 (root) or the UID the daemon is running as. - user = &UserState{Access: state.AdminAccess, UID: &ucred.Uid} + user = &UserState{Access: identities.AdminAccess, UID: &ucred.Uid} } else { // Regular read access if any other local UID. - user = &UserState{Access: state.ReadAccess, UID: &ucred.Uid} + user = &UserState{Access: identities.ReadAccess, UID: &ucred.Uid} } } diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go index aca315174..121b5ebd1 100644 --- a/internals/daemon/daemon_test.go +++ b/internals/daemon/daemon_test.go @@ -41,6 +41,7 @@ import ( "github.com/canonical/pebble/internals/logger" "github.com/canonical/pebble/internals/osutil" "github.com/canonical/pebble/internals/overlord" + "github.com/canonical/pebble/internals/overlord/identities" "github.com/canonical/pebble/internals/overlord/patch" "github.com/canonical/pebble/internals/overlord/restart" "github.com/canonical/pebble/internals/overlord/servstate" @@ -358,19 +359,20 @@ func (s *daemonSuite) testAccessChecker(c *C, tests []accessCheckerTestCase, rem d := s.newDaemon(c) // Add some named identities for testing with. + identitiesMgr := d.overlord.IdentitiesManager() d.state.Lock() - err := d.state.ReplaceIdentities(map[string]*state.Identity{ + err := identitiesMgr.ReplaceIdentities(map[string]*identities.Identity{ "adminuser": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1}, + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1}, }, "readuser": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 2}, + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 2}, }, "untrusteduser": { - Access: state.UntrustedAccess, - Local: &state.LocalIdentity{UserID: 3}, + Access: identities.UntrustedAccess, + Local: &identities.LocalIdentity{UserID: 3}, }, }) d.state.Unlock() @@ -562,7 +564,7 @@ func (s *daemonSuite) TestDefaultUcredUsers(c *C) { cmd.ServeHTTP(rec, req) c.Check(rec.Code, Equals, http.StatusOK) c.Assert(userSeen, NotNil) - c.Check(userSeen.Access, Equals, state.AdminAccess) + c.Check(userSeen.Access, Equals, identities.AdminAccess) c.Assert(userSeen.UID, NotNil) c.Check(*userSeen.UID, Equals, uint32(0)) @@ -576,7 +578,7 @@ func (s *daemonSuite) TestDefaultUcredUsers(c *C) { cmd.ServeHTTP(rec, req) c.Check(rec.Code, Equals, http.StatusOK) c.Assert(userSeen, NotNil) - c.Check(userSeen.Access, Equals, state.AdminAccess) + c.Check(userSeen.Access, Equals, identities.AdminAccess) c.Assert(userSeen.UID, NotNil) c.Check(*userSeen.UID, Equals, uint32(os.Getuid())) @@ -590,7 +592,7 @@ func (s *daemonSuite) TestDefaultUcredUsers(c *C) { cmd.ServeHTTP(rec, req) c.Check(rec.Code, Equals, http.StatusOK) c.Assert(userSeen, NotNil) - c.Check(userSeen.Access, Equals, state.ReadAccess) + c.Check(userSeen.Access, Equals, identities.ReadAccess) c.Assert(userSeen.UID, NotNil) c.Check(*userSeen.UID, Equals, uint32(os.Getuid()+1)) } @@ -1846,11 +1848,12 @@ func (s *daemonSuite) TestServeHTTPUserStateLocal(c *C) { d := s.newDaemon(c) // Set up a Local identity. + identitiesMgr := d.overlord.IdentitiesManager() d.state.Lock() - err := d.state.AddIdentities(map[string]*state.Identity{ + err := identitiesMgr.AddIdentities(map[string]*identities.Identity{ "localuser": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, }, }) d.state.Unlock() @@ -1880,7 +1883,7 @@ func (s *daemonSuite) TestServeHTTPUserStateLocal(c *C) { // Verify UserState for Local identity. c.Assert(capturedUser, NotNil) c.Assert(capturedUser.Username, Equals, "localuser") - c.Assert(capturedUser.Access, Equals, state.AdminAccess) + c.Assert(capturedUser.Access, Equals, identities.AdminAccess) c.Assert(capturedUser.UID, NotNil) // This specific expectation is only a temporary workaround to @@ -1917,7 +1920,7 @@ func (s *daemonSuite) TestServeHTTPUserStateUIDOnly(c *C) { // Verify UserState for UID-only (no named identity) c.Assert(capturedUser, NotNil) c.Assert(capturedUser.Username, Equals, "") - c.Assert(capturedUser.Access, Equals, state.ReadAccess) + c.Assert(capturedUser.Access, Equals, identities.ReadAccess) c.Assert(capturedUser.UID, NotNil) c.Assert(*capturedUser.UID, Equals, uint32(5000)) } @@ -1950,7 +1953,7 @@ func (s *daemonSuite) TestServeHTTPUserStateUIDOnlyRoot(c *C) { // Verify UserState for root UID. c.Assert(capturedUser, NotNil) c.Assert(capturedUser.Username, Equals, "") - c.Assert(capturedUser.Access, Equals, state.AdminAccess) + c.Assert(capturedUser.Access, Equals, identities.AdminAccess) c.Assert(capturedUser.UID, NotNil) c.Assert(*capturedUser.UID, Equals, uint32(0)) } @@ -1984,7 +1987,7 @@ func (s *daemonSuite) TestServeHTTPUserStateUIDOnlyDaemonUID(c *C) { // Verify UserState for daemon UID. c.Assert(capturedUser, NotNil) c.Assert(capturedUser.Username, Equals, "") - c.Assert(capturedUser.Access, Equals, state.AdminAccess) + c.Assert(capturedUser.Access, Equals, identities.AdminAccess) c.Assert(capturedUser.UID, NotNil) c.Assert(*capturedUser.UID, Equals, daemonUID) } diff --git a/internals/overlord/checkstate/handlers.go b/internals/overlord/checkstate/handlers.go index 42aa462fe..886ff6b7d 100644 --- a/internals/overlord/checkstate/handlers.go +++ b/internals/overlord/checkstate/handlers.go @@ -50,6 +50,7 @@ func (m *CheckManager) doPerformCheck(task *state.Task, tomb *tombpkg.Tomb) erro chk := newChecker(config) performCheck := func() (shouldExit bool, err error) { + //lint:ignore SA1012 providing a nil context to tomb.Context() is valid err = runCheck(tomb.Context(nil), chk, config.Timeout.Value) if !tomb.Alive() { return true, checkStopped(config.Name, task.Kind(), tomb.Err()) @@ -164,6 +165,7 @@ func (m *CheckManager) doRecoverCheck(task *state.Task, tomb *tombpkg.Tomb) erro chk := newChecker(config) recoverCheck := func() (shouldExit bool, err error) { + //lint:ignore SA1012 providing a nil context to tomb.Context() is valid err = runCheck(tomb.Context(nil), chk, config.Timeout.Value) if !tomb.Alive() { return true, checkStopped(config.Name, task.Kind(), tomb.Err()) diff --git a/internals/overlord/state/identities.go b/internals/overlord/identities/identities.go similarity index 63% rename from internals/overlord/state/identities.go rename to internals/overlord/identities/identities.go index 1bce0264d..c7d3b0907 100644 --- a/internals/overlord/state/identities.go +++ b/internals/overlord/identities/identities.go @@ -1,4 +1,4 @@ -// Copyright (c) 2024 Canonical Ltd +// Copyright (c) 2025 Canonical Ltd // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License version 3 as @@ -12,57 +12,99 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -package state +package identities import ( - "encoding/json" "errors" "fmt" "maps" "regexp" "sort" "strings" + + "github.com/canonical/pebble/internals/overlord/state" ) +const ( + identitiesKey = "identities" +) + +type Manager struct { + state *state.State + + // Keep a local copy to avoid having to deserialize from state each time + // Get is called. + identities map[string]*Identity +} + +func NewManager(st *state.State) (*Manager, error) { + m := &Manager{ + state: st, + identities: make(map[string]*Identity), + } + + m.state.Lock() + defer m.state.Unlock() + + // Read existing identities from state, if any. + err := st.Get(identitiesKey, &m.identities) + if err != nil && !errors.Is(err, state.ErrNoState) { + return nil, err + } + // Populate the Name field for each identity from the JSON object key. + for name, identity := range m.identities { + identity.Name = name + } + + return m, nil +} + +func (m *Manager) Ensure() error { + return nil +} + // Identity holds the configuration of a single identity. +// +// IMPORTANT: When adding a new identity type, if there's sensitive fields in it +// (like passwords), be sure to omit it from API marshalling in api_identities.go. type Identity struct { - Name string - Access IdentityAccess + Name string `json:"-"` + Access Access `json:"access"` // One or more of the following type-specific configuration fields must be // non-nil. - Local *LocalIdentity - Basic *BasicIdentity + Local *LocalIdentity `json:"local,omitempty"` + Basic *BasicIdentity `json:"basic,omitempty"` } -// IdentityAccess defines the access level for an identity. -type IdentityAccess string +// Access defines the access level for an identity. +type Access string const ( - AdminAccess IdentityAccess = "admin" - ReadAccess IdentityAccess = "read" - MetricsAccess IdentityAccess = "metrics" - UntrustedAccess IdentityAccess = "untrusted" + AdminAccess Access = "admin" + ReadAccess Access = "read" + MetricsAccess Access = "metrics" + UntrustedAccess Access = "untrusted" ) // LocalIdentity holds identity configuration specific to the "local" type // (for ucrednet/UID authentication). type LocalIdentity struct { - UserID uint32 + UserID uint32 `json:"user-id"` } // BasicIdentity holds identity configuration specific to the "basic" type // (for HTTP basic authentication). type BasicIdentity struct { // Password holds the user's sha512-crypt-hashed password. - Password string + Password string `json:"password"` } // This is used to ensure we send a well-formed identity Name. var identityNameRegexp = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9_\-]*$`) -// validate checks that the identity is valid, returning an error if not. -func (d *Identity) validate(name string) error { +// Validate checks that the identity's fields (and name) are valid, returning an error if not. +func (d *Identity) Validate(name string) error { if d == nil { return errors.New("identity must not be nil") } @@ -71,15 +113,6 @@ func (d *Identity) validate(name string) error { return fmt.Errorf("identity name %q invalid: must start with an alphabetic character and only contain alphanumeric characters, underscore, and hyphen", d.Name) } - return d.validateAccess() -} - -// validateAccess checks that the identity's access and type are valid, returning an error if not. -func (d *Identity) validateAccess() error { - if d == nil { - return errors.New("identity must not be nil") - } - switch d.Access { case AdminAccess, ReadAccess, MetricsAccess, UntrustedAccess: case "": @@ -107,85 +140,18 @@ func (d *Identity) validateAccess() error { return nil } -// apiIdentity exists so the default JSON marshalling of an Identity (used -// for API responses) excludes secrets. The marshalledIdentity type is used -// for saving secrets in state. -type apiIdentity struct { - Access string `json:"access"` - Local *apiLocalIdentity `json:"local,omitempty"` - Basic *apiBasicIdentity `json:"basic,omitempty"` - Cert *apiCertIdentity `json:"cert,omitempty"` -} - -type apiLocalIdentity struct { - UserID *uint32 `json:"user-id"` -} - -type apiBasicIdentity struct { - Password string `json:"password"` -} - -type apiCertIdentity struct { - PEM string `json:"pem"` -} - -// IMPORTANT NOTE: be sure to exclude secrets when adding to this! -func (d *Identity) MarshalJSON() ([]byte, error) { - ai := apiIdentity{ - Access: string(d.Access), - } - if d.Local != nil { - ai.Local = &apiLocalIdentity{UserID: &d.Local.UserID} - } - if d.Basic != nil { - ai.Basic = &apiBasicIdentity{Password: "*****"} - } - return json.Marshal(ai) -} - -func (d *Identity) UnmarshalJSON(data []byte) error { - var ai apiIdentity - err := json.Unmarshal(data, &ai) - if err != nil { - return err - } - - identity := Identity{ - Access: IdentityAccess(ai.Access), - } - - if ai.Local != nil { - if ai.Local.UserID == nil { - return errors.New("local identity must specify user-id") - } - identity.Local = &LocalIdentity{UserID: *ai.Local.UserID} - } - if ai.Basic != nil { - identity.Basic = &BasicIdentity{Password: ai.Basic.Password} - } - - // Perform additional validation using the local Identity type. - err = identity.validateAccess() - if err != nil { - return err - } - - *d = identity - return nil -} - // AddIdentities adds the given identities to the system. It's an error if any // of the named identities already exist. -func (s *State) AddIdentities(identities map[string]*Identity) error { - s.reading() - +// +// The state lock must be held for the duration of this call. +func (m *Manager) AddIdentities(identities map[string]*Identity) error { // If any of the named identities already exist, return an error. var existing []string for name, identity := range identities { - if _, ok := s.identities[name]; ok { + if _, ok := m.identities[name]; ok { existing = append(existing, name) } - err := identity.validate(name) + err := identity.Validate(name) if err != nil { return fmt.Errorf("identity %q invalid: %w", name, err) } @@ -195,7 +161,7 @@ func (s *State) AddIdentities(identities map[string]*Identity) error { return fmt.Errorf("identities already exist: %s", strings.Join(existing, ", ")) } - newIdentities := maps.Clone(s.identities) + newIdentities := maps.Clone(m.identities) for name, identity := range identities { identity.Name = name newIdentities[name] = identity @@ -206,23 +172,23 @@ func (s *State) AddIdentities(identities map[string]*Identity) error { return err } - s.writing() - s.identities = newIdentities + m.identities = newIdentities + m.state.Set(identitiesKey, newIdentities) return nil } // UpdateIdentities updates the given identities in the system. It's an error // if any of the named identities do not exist. -func (s *State) UpdateIdentities(identities map[string]*Identity) error { - s.reading() - +// +// The state lock must be held for the duration of this call. +func (m *Manager) UpdateIdentities(identities map[string]*Identity) error { // If any of the named identities don't exist, return an error. var missing []string for name, identity := range identities { - if _, ok := s.identities[name]; !ok { + if _, ok := m.identities[name]; !ok { missing = append(missing, name) } - err := identity.validate(name) + err := identity.Validate(name) if err != nil { return fmt.Errorf("identity %q invalid: %w", name, err) } @@ -232,7 +198,7 @@ func (s *State) UpdateIdentities(identities map[string]*Identity) error { return fmt.Errorf("identities do not exist: %s", strings.Join(missing, ", ")) } - newIdentities := maps.Clone(s.identities) + newIdentities := maps.Clone(m.identities) for name, identity := range identities { identity.Name = name newIdentities[name] = identity @@ -243,27 +209,27 @@ func (s *State) UpdateIdentities(identities map[string]*Identity) error { return err } - s.writing() - s.identities = newIdentities + m.identities = newIdentities + m.state.Set(identitiesKey, newIdentities) return nil } // ReplaceIdentities replaces the named identities in the system with the // given identities (adding those that don't exist), or removes them if the // map value is nil. -func (s *State) ReplaceIdentities(identities map[string]*Identity) error { - s.reading() - +// +// The state lock must be held for the duration of this call. +func (m *Manager) ReplaceIdentities(identities map[string]*Identity) error { for name, identity := range identities { if identity != nil { - err := identity.validate(name) + err := identity.Validate(name) if err != nil { return fmt.Errorf("identity %q invalid: %w", name, err) } } } - newIdentities := maps.Clone(s.identities) + newIdentities := maps.Clone(m.identities) for name, identity := range identities { if identity == nil { delete(newIdentities, name) @@ -278,20 +244,20 @@ func (s *State) ReplaceIdentities(identities map[string]*Identity) error { return err } - s.writing() - s.identities = newIdentities + m.identities = newIdentities + m.state.Set(identitiesKey, newIdentities) return nil } // RemoveIdentities removes the named identities from the system. It's an // error if any of the named identities do not exist. -func (s *State) RemoveIdentities(identities map[string]struct{}) error { - s.reading() - +// +// The state lock must be held for the duration of this call. +func (m *Manager) RemoveIdentities(identities map[string]struct{}) error { // If any of the named identities don't exist, return an error. var missing []string for name := range identities { - if _, ok := s.identities[name]; !ok { + if _, ok := m.identities[name]; !ok { missing = append(missing, name) } } @@ -300,34 +266,32 @@ func (s *State) RemoveIdentities(identities map[string]struct{}) error { return fmt.Errorf("identities do not exist: %s", strings.Join(missing, ", ")) } - s.writing() for name := range identities { - delete(s.identities, name) + delete(m.identities, name) } + m.state.Set(identitiesKey, m.identities) return nil } // Identities returns all the identities in the system. The returned map is a // shallow clone, so map mutations won't affect state. -func (s *State) Identities() map[string]*Identity { - s.reading() - - return maps.Clone(s.identities) +// +// The state lock must be held for the duration of this call. +func (m *Manager) Identities() map[string]*Identity { + return maps.Clone(m.identities) } // IdentityFromInputs returns an identity matching the given inputs. // // If no matching identity is found for the given inputs, nil is returned. -func (s *State) IdentityFromInputs(userID *uint32, username, password string) *Identity { - s.reading() - +func (m *Manager) IdentityFromInputs(userID *uint32, username, password string) *Identity { switch { case username != "" || password != "": // Password validation is disabled in FIPS builds, so we bail. return nil case userID != nil: - for _, identity := range s.identities { + for _, identity := range m.identities { if identity.Local != nil && identity.Local.UserID == *userID { return identity } diff --git a/internals/overlord/identities/identities_test.go b/internals/overlord/identities/identities_test.go new file mode 100644 index 000000000..392d9887d --- /dev/null +++ b/internals/overlord/identities/identities_test.go @@ -0,0 +1,644 @@ +// Copyright (c) 2024 Canonical Ltd +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 3 as +// published by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package identities_test + +import ( + "bytes" + "encoding/json" + "testing" + + . "gopkg.in/check.v1" + + "github.com/canonical/pebble/internals/overlord/identities" + "github.com/canonical/pebble/internals/overlord/state" +) + +func TestIdentities(t *testing.T) { TestingT(t) } + +type identitiesSuite struct{} + +var _ = Suite(&identitiesSuite{}) + +func (s *identitiesSuite) TestMarshalState(c *C) { + st := state.New(nil) + mgr, err := identities.NewManager(st) + c.Assert(err, IsNil) + + st.Lock() + defer st.Unlock() + + err = mgr.AddIdentities(map[string]*identities.Identity{ + "bob": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "mary": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + }) + c.Assert(err, IsNil) + + // Marshal entire state, then pull out just the "identities" key to test that. + data, err := json.Marshal(st) + c.Assert(err, IsNil) + + var unmarshalled map[string]any + err = json.Unmarshal(data, &unmarshalled) + c.Assert(err, IsNil) + customData := unmarshalled["data"].(map[string]any) + + data, err = json.MarshalIndent(customData["identities"], "", " ") + c.Assert(err, IsNil) + c.Assert(string(data), Equals, ` +{ + "bob": { + "access": "read", + "local": { + "user-id": 42 + } + }, + "mary": { + "access": "admin", + "local": { + "user-id": 1000 + } + } +}`[1:]) + + _, hasLegacyIdentities := unmarshalled["identities"] + c.Assert(hasLegacyIdentities, Equals, false) +} + +func (s *identitiesSuite) TestUnmarshalState(c *C) { + data := []byte(` +{ + "data": { + "identities": { + "bob": { + "access": "read", + "local": { + "user-id": 42 + } + }, + "mary": { + "access": "admin", + "local": { + "user-id": 1000 + } + } + } + } +}`) + + st, err := state.ReadState(nil, bytes.NewReader(data)) + c.Assert(err, IsNil) + mgr, err := identities.NewManager(st) + c.Assert(err, IsNil) + + st.Lock() + defer st.Unlock() + + c.Assert(mgr.Identities(), DeepEquals, map[string]*identities.Identity{ + "bob": { + Name: "bob", + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "mary": { + Name: "mary", + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + }) +} + +func (s *identitiesSuite) TestAddIdentities(c *C) { + st := state.New(nil) + mgr, err := identities.NewManager(st) + c.Assert(err, IsNil) + + st.Lock() + defer st.Unlock() + + original := map[string]*identities.Identity{ + "bob": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "mary": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + "nancy": { + Access: identities.MetricsAccess, + Basic: &identities.BasicIdentity{Password: "hash"}, + }, + } + err = mgr.AddIdentities(original) + c.Assert(err, IsNil) + + // Ensure they were added correctly (and Name fields have been set). + idents := mgr.Identities() + c.Assert(idents, DeepEquals, map[string]*identities.Identity{ + "bob": { + Name: "bob", + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "mary": { + Name: "mary", + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + "nancy": { + Name: "nancy", + Access: identities.MetricsAccess, + Basic: &identities.BasicIdentity{Password: "hash"}, + }, + }) + + // Can't add identity names that already exist. + err = mgr.AddIdentities(map[string]*identities.Identity{ + "bill": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 43}, + }, + "bob": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "mary": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + }) + c.Assert(err, ErrorMatches, "identities already exist: bob, mary") + + // Can't add a nil identity. + err = mgr.AddIdentities(map[string]*identities.Identity{ + "bill": nil, + }) + c.Assert(err, ErrorMatches, `identity "bill" invalid: identity must not be nil`) + + // Access value must be valid. + err = mgr.AddIdentities(map[string]*identities.Identity{ + "bill": { + Access: "bar", + Local: &identities.LocalIdentity{UserID: 43}, + }, + }) + c.Assert(err, ErrorMatches, `identity "bill" invalid: invalid access value "bar", must be "admin", "read", "metrics", or "untrusted"`) + + // Must have at least one type. + err = mgr.AddIdentities(map[string]*identities.Identity{ + "bill": { + Access: "admin", + }, + }) + c.Assert(err, ErrorMatches, `identity "bill" invalid: identity must have at least one type \("local", "basic", or "cert"\)`) + + // May have two types. + err = mgr.AddIdentities(map[string]*identities.Identity{ + "peter": { + Access: identities.MetricsAccess, + Basic: &identities.BasicIdentity{Password: "hash"}, + Local: &identities.LocalIdentity{UserID: 1001}, + }, + }) + c.Assert(err, IsNil) + + // Ensure user IDs are unique with existing users. + err = mgr.AddIdentities(map[string]*identities.Identity{ + "bill": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + }) + c.Assert(err, ErrorMatches, `cannot have multiple identities with user ID 1000 \(bill, mary\)`) + + // Ensure user IDs are unique among the ones being added (and test >2 with same UID). + err = mgr.AddIdentities(map[string]*identities.Identity{ + "bill": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 2000}, + }, + "bale": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 2000}, + }, + "boll": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 2000}, + }, + }) + c.Assert(err, ErrorMatches, `cannot have multiple identities with user ID 2000 \(bale, bill, boll\)`) +} + +func (s *identitiesSuite) TestUpdateIdentities(c *C) { + st := state.New(nil) + mgr, err := identities.NewManager(st) + c.Assert(err, IsNil) + + st.Lock() + defer st.Unlock() + + original := map[string]*identities.Identity{ + "bob": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "mary": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + "nancy": { + Access: identities.MetricsAccess, + Basic: &identities.BasicIdentity{Password: "hash"}, + }, + } + err = mgr.AddIdentities(original) + c.Assert(err, IsNil) + + err = mgr.UpdateIdentities(map[string]*identities.Identity{ + "bob": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + "mary": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "nancy": { + Access: identities.MetricsAccess, + Basic: &identities.BasicIdentity{Password: "new hash"}, + }, + }) + c.Assert(err, IsNil) + + // Ensure they were updated correctly. + idents := mgr.Identities() + c.Assert(idents, DeepEquals, map[string]*identities.Identity{ + "bob": { + Name: "bob", + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + "mary": { + Name: "mary", + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "nancy": { + Name: "nancy", + Access: identities.MetricsAccess, + Basic: &identities.BasicIdentity{Password: "new hash"}, + }, + }) + + // Can't update identity names that don't exist. + err = mgr.UpdateIdentities(map[string]*identities.Identity{ + "bill": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 43}, + }, + "bale": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "mary": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + }) + c.Assert(err, ErrorMatches, "identities do not exist: bale, bill") + + // Ensure validation is being done (full testing done in AddIdentity). + err = mgr.UpdateIdentities(map[string]*identities.Identity{ + "bob": nil, + }) + c.Assert(err, ErrorMatches, `identity "bob" invalid: identity must not be nil`) + + // Ensure unique user ID testing is being done (full testing done in AddIdentity). + err = mgr.UpdateIdentities(map[string]*identities.Identity{ + "bob": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + }) + c.Assert(err, ErrorMatches, `cannot have multiple identities with user ID 42 \(bob, mary\)`) +} + +func (s *identitiesSuite) TestReplaceIdentities(c *C) { + st := state.New(nil) + mgr, err := identities.NewManager(st) + c.Assert(err, IsNil) + + st.Lock() + defer st.Unlock() + + original := map[string]*identities.Identity{ + "bob": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "mary": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + } + err = mgr.AddIdentities(original) + c.Assert(err, IsNil) + + err = mgr.ReplaceIdentities(map[string]*identities.Identity{ + "bob": nil, // nil means remove it + "mary": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 43}, + }, + "newguy": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 44}, + }, + }) + c.Assert(err, IsNil) + + // Ensure they were added/updated/deleted correctly. + idents := mgr.Identities() + c.Assert(idents, DeepEquals, map[string]*identities.Identity{ + "mary": { + Name: "mary", + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 43}, + }, + "newguy": { + Name: "newguy", + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 44}, + }, + }) + + // Ensure validation is being done (full testing done in AddIdentity). + err = mgr.ReplaceIdentities(map[string]*identities.Identity{ + "bill": { + Access: "admin", + }, + }) + c.Assert(err, ErrorMatches, `identity "bill" invalid: identity must have at least one type \("local", "basic", or "cert"\)`) + + // Ensure unique user ID testing is being done (full testing done in AddIdentity). + err = mgr.ReplaceIdentities(map[string]*identities.Identity{ + "bob": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 43}, + }, + }) + c.Assert(err, ErrorMatches, `cannot have multiple identities with user ID 43 \(bob, mary\)`) +} + +func (s *identitiesSuite) TestRemoveIdentities(c *C) { + st := state.New(nil) + mgr, err := identities.NewManager(st) + c.Assert(err, IsNil) + + st.Lock() + defer st.Unlock() + + original := map[string]*identities.Identity{ + "bill": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 43}, + }, + "bob": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "mary": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + "nancy": { + Access: identities.MetricsAccess, + Basic: &identities.BasicIdentity{Password: "hash"}, + }, + "queen": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1001}, + }, + } + err = mgr.AddIdentities(original) + c.Assert(err, IsNil) + + err = mgr.RemoveIdentities(map[string]struct{}{ + "bob": {}, + "mary": {}, + "nancy": {}, + }) + c.Assert(err, IsNil) + + // Ensure they were removed correctly. + idents := mgr.Identities() + c.Assert(idents, DeepEquals, map[string]*identities.Identity{ + "bill": { + Name: "bill", + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 43}, + }, + "queen": { + Name: "queen", + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1001}, + }, + }) + + // Can't remove identity names that don't exist. + err = mgr.RemoveIdentities(map[string]struct{}{ + "bill": {}, + "bale": {}, + "mary": {}, + }) + c.Assert(err, ErrorMatches, "identities do not exist: bale, mary") +} + +func (s *identitiesSuite) TestIdentities(c *C) { + st := state.New(nil) + mgr, err := identities.NewManager(st) + c.Assert(err, IsNil) + + st.Lock() + defer st.Unlock() + + original := map[string]*identities.Identity{ + "bob": { + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "mary": { + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + "nancy": { + Access: identities.MetricsAccess, + Basic: &identities.BasicIdentity{Password: "hash"}, + }, + } + err = mgr.AddIdentities(original) + c.Assert(err, IsNil) + + // Ensure it returns correct results. + idents := mgr.Identities() + expected := map[string]*identities.Identity{ + "bob": { + Name: "bob", + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "mary": { + Name: "mary", + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + "nancy": { + Name: "nancy", + Access: identities.MetricsAccess, + Basic: &identities.BasicIdentity{Password: "hash"}, + }, + } + c.Assert(idents, DeepEquals, expected) + + // Ensure the map was cloned (mutations to first map won't affect second). + idents2 := mgr.Identities() + c.Assert(idents2, DeepEquals, expected) + idents["changed"] = &identities.Identity{} + c.Assert(idents2, DeepEquals, expected) +} + +func (s *identitiesSuite) TestIdentityFromInputs(c *C) { + st := state.New(nil) + mgr, err := identities.NewManager(st) + c.Assert(err, IsNil) + + st.Lock() + defer st.Unlock() + + ids := map[string]*identities.Identity{ + "uid": { + Access: identities.MetricsAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "basic": { + Access: identities.ReadAccess, + Basic: &identities.BasicIdentity{ + // password: test + Password: "$6$F9cFSVEKyO4gB1Wh$8S1BSKsNkF.jBAixGc4W7l80OpfCNk65LZBDHBng3NAmbcHuMj4RIm7992rrJ8YA.SJ0hvm.vGk2z483am4Ym1", + }, + }, + } + err = mgr.AddIdentities(ids) + c.Assert(err, IsNil) + + tests := []struct { + name string + userID *uint32 + basicUser string + basicPass string + expectedUser string + expectedAccess identities.Access + }{{ + name: "no inputs", + expectedUser: "", + }, { + // Basic authentication tests (medium priority) + name: "valid basic auth", + basicUser: "basic", + basicPass: "test", + expectedUser: "", // disabled in FIPS builds + }, { + name: "valid user invalid password", + basicUser: "basic", + basicPass: "wrong", + expectedUser: "", + }, { + name: "invalid user valid password", + basicUser: "nonexistent", + basicPass: "test", + expectedUser: "", + }, { + name: "invalid user invalid password", + basicUser: "nonexistent", + basicPass: "wrong", + expectedUser: "", + }, { + name: "empty user with password", + basicUser: "", + basicPass: "test", + expectedUser: "", + }, { + name: "user with empty password", + basicUser: "basic", + basicPass: "", + expectedUser: "", + }, { + name: "empty user and password", + basicUser: "", + basicPass: "", + expectedUser: "", + }, { + // Basic auth overrides UID + name: "basic auth with uid ignored", + basicUser: "basic", + basicPass: "test", + userID: ptr(uint32(42)), + expectedUser: "", // disabled in FIPS builds + }, { + name: "invalid basic auth with valid uid ignored", + basicUser: "basic", + basicPass: "wrong", + userID: ptr(uint32(42)), + expectedUser: "", + }, { + // Local/UID authentication tests (lowest priority) + name: "valid uid", + userID: ptr(uint32(42)), + expectedUser: "uid", + expectedAccess: identities.MetricsAccess, + }, { + name: "invalid uid", + userID: ptr(uint32(100)), + expectedUser: "", + }, { + // Edge cases + name: "nil uid", + userID: nil, + expectedUser: "", + }} + + for _, test := range tests { + c.Logf("Running test: %s", test.name) + identity := mgr.IdentityFromInputs(test.userID, test.basicUser, test.basicPass) + + if test.expectedUser != "" { + c.Assert(identity, NotNil) + c.Assert(identity.Name, Equals, test.expectedUser) + c.Assert(identity.Access, Equals, test.expectedAccess) + } else { + c.Assert(identity, IsNil) + } + } +} + +func ptr[T any](v T) *T { + return &v +} diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go index d591f5dd3..b9b458f76 100644 --- a/internals/overlord/overlord.go +++ b/internals/overlord/overlord.go @@ -32,6 +32,7 @@ import ( "github.com/canonical/pebble/internals/osutil" "github.com/canonical/pebble/internals/overlord/checkstate" "github.com/canonical/pebble/internals/overlord/cmdstate" + "github.com/canonical/pebble/internals/overlord/identities" "github.com/canonical/pebble/internals/overlord/logstate" "github.com/canonical/pebble/internals/overlord/patch" "github.com/canonical/pebble/internals/overlord/planstate" @@ -111,15 +112,16 @@ type Overlord struct { startOfOperationTime time.Time // managers - inited bool - startedUp bool - runner *state.TaskRunner - restartMgr *restart.RestartManager - planMgr *planstate.PlanManager - serviceMgr *servstate.ServiceManager - commandMgr *cmdstate.CommandManager - checkMgr *checkstate.CheckManager - logMgr *logstate.LogManager + inited bool + startedUp bool + runner *state.TaskRunner + restartMgr *restart.RestartManager + planMgr *planstate.PlanManager + serviceMgr *servstate.ServiceManager + commandMgr *cmdstate.CommandManager + checkMgr *checkstate.CheckManager + logMgr *logstate.LogManager + identitiesMgr *identities.Manager extension Extension } @@ -194,6 +196,12 @@ func New(opts *Options) (*Overlord, error) { } o.stateEng.AddManager(o.planMgr) + o.identitiesMgr, err = identities.NewManager(s) + if err != nil { + return nil, fmt.Errorf("cannot create identities manager: %w", err) + } + o.stateEng.AddManager(o.identitiesMgr) + o.logMgr = logstate.NewLogManager() o.serviceMgr, err = servstate.NewManager( @@ -621,6 +629,12 @@ func (o *Overlord) PlanManager() *planstate.PlanManager { return o.planMgr } +// IdentitiesManager returns the manager responsible for managing client +// identities. +func (o *Overlord) IdentitiesManager() *identities.Manager { + return o.identitiesMgr +} + // Fake creates an Overlord without any managers and with a backend // not using disk. Managers can be added with AddManager. For testing. func Fake() *Overlord { diff --git a/internals/overlord/patch/patch.go b/internals/overlord/patch/patch.go index 58514a8a4..c40d90ee3 100644 --- a/internals/overlord/patch/patch.go +++ b/internals/overlord/patch/patch.go @@ -28,7 +28,7 @@ import ( ) // Level is the current implemented patch level of the state format and content. -var Level = 1 +var Level = 2 // Sublevel is the current implemented sublevel for the Level. // Sublevel 0 is the first patch for the new Level, rollback below x.0 is not possible. diff --git a/internals/overlord/patch/patch2.go b/internals/overlord/patch/patch2.go new file mode 100644 index 000000000..7cf058f29 --- /dev/null +++ b/internals/overlord/patch/patch2.go @@ -0,0 +1,49 @@ +// Copyright (c) 2025 Canonical Ltd +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 3 as +// published by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package patch + +import ( + "encoding/json" + "fmt" + + "github.com/canonical/pebble/internals/logger" + "github.com/canonical/pebble/internals/overlord/identities" + "github.com/canonical/pebble/internals/overlord/state" +) + +func init() { + patches[2] = []PatchFunc{patch2} +} + +// Load legacy identities if present (and new identities are not in Get/Set data). +func patch2(s *state.State) error { + legacy := s.LegacyIdentities() + if len(legacy) == 0 { + return nil + } + if s.Has("identities") { + logger.Noticef("WARNING: both new and legacy identities found in state file, ignoring legacy") + return nil + } + + var idents map[string]*identities.Identity + err := json.Unmarshal(legacy, &idents) + if err != nil { + return fmt.Errorf("cannot unmarshal legacy identities: %v", err) + } + s.Set("identities", idents) + logger.Noticef("Loaded legacy identities from state file") + return nil +} diff --git a/internals/overlord/patch/patch2_test.go b/internals/overlord/patch/patch2_test.go new file mode 100644 index 000000000..034f06edd --- /dev/null +++ b/internals/overlord/patch/patch2_test.go @@ -0,0 +1,133 @@ +// Copyright (c) 2026 Canonical Ltd +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 3 as +// published by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package patch_test + +import ( + "bytes" + + . "gopkg.in/check.v1" + + "github.com/canonical/pebble/internals/overlord/identities" + "github.com/canonical/pebble/internals/overlord/patch" + "github.com/canonical/pebble/internals/overlord/state" +) + +type patch2Suite struct{} + +var _ = Suite(&patch2Suite{}) + +func (s *patch2Suite) TestLegacyIdentities(c *C) { + restore := patch.FakeLevel(2, 1) + defer restore() + + data := []byte(` +{ + "data": {"patch-level": 1}, + "identities": { + "bob": { + "access": "read", + "local": { + "user-id": 42 + } + }, + "mary": { + "access": "admin", + "local": { + "user-id": 1000 + } + } + } +}`) + + st, err := state.ReadState(nil, bytes.NewReader(data)) + c.Assert(err, IsNil) + err = patch.Apply(st) + c.Assert(err, IsNil) + mgr, err := identities.NewManager(st) + c.Assert(err, IsNil) + + st.Lock() + defer st.Unlock() + + c.Assert(mgr.Identities(), DeepEquals, map[string]*identities.Identity{ + "bob": { + Name: "bob", + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + "mary": { + Name: "mary", + Access: identities.AdminAccess, + Local: &identities.LocalIdentity{UserID: 1000}, + }, + }) + + // ensure we moved forward to patch-level 2 (sublevel 0) + var patchLevel int + err = st.Get("patch-level", &patchLevel) + c.Assert(err, IsNil) + c.Assert(patchLevel, Equals, 2) + err = st.Get("patch-sublevel", &patchLevel) + c.Assert(err, IsNil) + c.Assert(patchLevel, Equals, 0) +} + +func (s *patch2Suite) TestNewAndLegacyIdentities(c *C) { + restore := patch.FakeLevel(2, 1) + defer restore() + + // If both new and legacy are present, it should prefer the new + // (and emit a warning log, but we don't test for that). + data := []byte(` +{ + "data": { + "patch-level": 1, + "identities": { + "bob": { + "access": "read", + "local": { + "user-id": 42 + } + } + } + }, + "identities": { + "mary": { + "access": "admin", + "local": { + "user-id": 1000 + } + } + } +}`) + + st, err := state.ReadState(nil, bytes.NewReader(data)) + c.Assert(err, IsNil) + err = patch.Apply(st) + c.Assert(err, IsNil) + mgr, err := identities.NewManager(st) + c.Assert(err, IsNil) + + st.Lock() + defer st.Unlock() + + c.Assert(mgr.Identities(), DeepEquals, map[string]*identities.Identity{ + "bob": { + Name: "bob", + Access: identities.ReadAccess, + Local: &identities.LocalIdentity{UserID: 42}, + }, + }) +} diff --git a/internals/overlord/state/identities_test.go b/internals/overlord/state/identities_test.go deleted file mode 100644 index c30f127a3..000000000 --- a/internals/overlord/state/identities_test.go +++ /dev/null @@ -1,724 +0,0 @@ -// Copyright (c) 2024 Canonical Ltd -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License version 3 as -// published by the Free Software Foundation. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -package state_test - -import ( - "encoding/json" - - . "gopkg.in/check.v1" - - "github.com/canonical/pebble/internals/overlord/state" -) - -type identitiesSuite struct{} - -var _ = Suite(&identitiesSuite{}) - -// IMPORTANT NOTE: be sure secrets aren't included when adding to this! -func (s *identitiesSuite) TestMarshalAPI(c *C) { - st := state.New(nil) - st.Lock() - defer st.Unlock() - - err := st.AddIdentities(map[string]*state.Identity{ - "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - "nancy": { - Access: state.MetricsAccess, - Basic: &state.BasicIdentity{Password: "hash"}, - }, - }) - c.Assert(err, IsNil) - - identities := st.Identities() - data, err := json.MarshalIndent(identities, "", " ") - c.Assert(err, IsNil) - c.Assert(string(data), Equals, ` -{ - "bob": { - "access": "read", - "local": { - "user-id": 42 - } - }, - "mary": { - "access": "admin", - "local": { - "user-id": 1000 - } - }, - "nancy": { - "access": "metrics", - "basic": { - "password": "*****" - } - } -}`[1:]) -} - -func (s *identitiesSuite) TestUnmarshalAPI(c *C) { - data := []byte(` -{ - "bob": { - "access": "read", - "local": { - "user-id": 42 - } - }, - "mary": { - "access": "admin", - "local": { - "user-id": 1000 - } - }, - "nancy": { - "access": "metrics", - "basic": { - "password": "hash" - } - } -}`) - var identities map[string]*state.Identity - err := json.Unmarshal(data, &identities) - c.Assert(err, IsNil) - c.Assert(identities, DeepEquals, map[string]*state.Identity{ - "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - "nancy": { - Access: state.MetricsAccess, - Basic: &state.BasicIdentity{Password: "hash"}, - }, - }) -} - -func (s *identitiesSuite) TestUnmarshalAPIErrors(c *C) { - tests := []struct { - data string - error string - }{{ - data: `{"no-type": {"access": "admin"}}`, - error: `identity must have at least one type \("local", "basic", or "cert"\)`, - }, { - data: `{"invalid-access": {"access": "admin", "local": {}}}`, - error: `local identity must specify user-id`, - }, { - data: `{"invalid-access": {"access": "metrics", "basic": {}}}`, - error: `basic identity must specify password \(hashed\)`, - }, { - data: `{"invalid-access": {"access": "foo", "local": {"user-id": 42}}}`, - error: `invalid access value "foo", must be "admin", "read", "metrics", or "untrusted"`, - }, { - data: `{"invalid-access": {"local": {"user-id": 42}}}`, - error: `access value must be specified \("admin", "read", "metrics", or "untrusted"\)`, - }} - for _, test := range tests { - c.Logf("Input data: %s", test.data) - var identities map[string]*state.Identity - err := json.Unmarshal([]byte(test.data), &identities) - c.Check(err, ErrorMatches, test.error) - } -} - -func (s *identitiesSuite) TestMarshalState(c *C) { - st := state.New(nil) - st.Lock() - defer st.Unlock() - - err := st.AddIdentities(map[string]*state.Identity{ - "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - }) - c.Assert(err, IsNil) - - // Marshal entire state, then pull out just the "identities" key to test that. - data, err := json.Marshal(st) - c.Assert(err, IsNil) - var unmarshalled map[string]any - err = json.Unmarshal(data, &unmarshalled) - c.Assert(err, IsNil) - data, err = json.MarshalIndent(unmarshalled["identities"], "", " ") - c.Assert(err, IsNil) - c.Assert(string(data), Equals, ` -{ - "bob": { - "access": "read", - "local": { - "user-id": 42 - } - }, - "mary": { - "access": "admin", - "local": { - "user-id": 1000 - } - } -}`[1:]) -} - -func (s *identitiesSuite) TestUnmarshalState(c *C) { - st := state.New(nil) - st.Lock() - defer st.Unlock() - - data := []byte(` -{ - "identities": { - "bob": { - "access": "read", - "local": { - "user-id": 42 - } - }, - "mary": { - "access": "admin", - "local": { - "user-id": 1000 - } - } - } -}`) - err := json.Unmarshal(data, &st) - c.Assert(err, IsNil) - c.Assert(st.Identities(), DeepEquals, map[string]*state.Identity{ - "bob": { - Name: "bob", - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Name: "mary", - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - }) -} - -func (s *identitiesSuite) TestAddIdentities(c *C) { - st := state.New(nil) - st.Lock() - defer st.Unlock() - - original := map[string]*state.Identity{ - "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - "nancy": { - Access: state.MetricsAccess, - Basic: &state.BasicIdentity{Password: "hash"}, - }, - } - err := st.AddIdentities(original) - c.Assert(err, IsNil) - - // Ensure they were added correctly (and Name fields have been set). - identities := st.Identities() - c.Assert(identities, DeepEquals, map[string]*state.Identity{ - "bob": { - Name: "bob", - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Name: "mary", - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - "nancy": { - Name: "nancy", - Access: state.MetricsAccess, - Basic: &state.BasicIdentity{Password: "hash"}, - }, - }) - - // Can't add identity names that already exist. - err = st.AddIdentities(map[string]*state.Identity{ - "bill": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 43}, - }, - "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - }) - c.Assert(err, ErrorMatches, "identities already exist: bob, mary") - - // Can't add a nil identity. - err = st.AddIdentities(map[string]*state.Identity{ - "bill": nil, - }) - c.Assert(err, ErrorMatches, `identity "bill" invalid: identity must not be nil`) - - // Access value must be valid. - err = st.AddIdentities(map[string]*state.Identity{ - "bill": { - Access: "bar", - Local: &state.LocalIdentity{UserID: 43}, - }, - }) - c.Assert(err, ErrorMatches, `identity "bill" invalid: invalid access value "bar", must be "admin", "read", "metrics", or "untrusted"`) - - // Must have at least one type. - err = st.AddIdentities(map[string]*state.Identity{ - "bill": { - Access: "admin", - }, - }) - c.Assert(err, ErrorMatches, `identity "bill" invalid: identity must have at least one type \("local", "basic", or "cert"\)`) - - // May have two types. - err = st.AddIdentities(map[string]*state.Identity{ - "peter": { - Access: state.MetricsAccess, - Basic: &state.BasicIdentity{Password: "hash"}, - Local: &state.LocalIdentity{UserID: 1001}, - }, - }) - c.Assert(err, IsNil) - - // Ensure user IDs are unique with existing users. - err = st.AddIdentities(map[string]*state.Identity{ - "bill": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - }) - c.Assert(err, ErrorMatches, `cannot have multiple identities with user ID 1000 \(bill, mary\)`) - - // Ensure user IDs are unique among the ones being added (and test >2 with same UID). - err = st.AddIdentities(map[string]*state.Identity{ - "bill": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 2000}, - }, - "bale": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 2000}, - }, - "boll": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 2000}, - }, - }) - c.Assert(err, ErrorMatches, `cannot have multiple identities with user ID 2000 \(bale, bill, boll\)`) -} - -func (s *identitiesSuite) TestUpdateIdentities(c *C) { - st := state.New(nil) - st.Lock() - defer st.Unlock() - - original := map[string]*state.Identity{ - "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - "nancy": { - Access: state.MetricsAccess, - Basic: &state.BasicIdentity{Password: "hash"}, - }, - } - err := st.AddIdentities(original) - c.Assert(err, IsNil) - - err = st.UpdateIdentities(map[string]*state.Identity{ - "bob": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - "mary": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "nancy": { - Access: state.MetricsAccess, - Basic: &state.BasicIdentity{Password: "new hash"}, - }, - }) - c.Assert(err, IsNil) - - // Ensure they were updated correctly. - identities := st.Identities() - c.Assert(identities, DeepEquals, map[string]*state.Identity{ - "bob": { - Name: "bob", - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - "mary": { - Name: "mary", - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "nancy": { - Name: "nancy", - Access: state.MetricsAccess, - Basic: &state.BasicIdentity{Password: "new hash"}, - }, - }) - - // Can't update identity names that don't exist. - err = st.UpdateIdentities(map[string]*state.Identity{ - "bill": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 43}, - }, - "bale": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - }) - c.Assert(err, ErrorMatches, "identities do not exist: bale, bill") - - // Ensure validation is being done (full testing done in AddIdentity). - err = st.UpdateIdentities(map[string]*state.Identity{ - "bob": nil, - }) - c.Assert(err, ErrorMatches, `identity "bob" invalid: identity must not be nil`) - - // Ensure unique user ID testing is being done (full testing done in AddIdentity). - err = st.UpdateIdentities(map[string]*state.Identity{ - "bob": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - }) - c.Assert(err, ErrorMatches, `cannot have multiple identities with user ID 42 \(bob, mary\)`) -} - -func (s *identitiesSuite) TestReplaceIdentities(c *C) { - st := state.New(nil) - st.Lock() - defer st.Unlock() - - original := map[string]*state.Identity{ - "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - } - err := st.AddIdentities(original) - c.Assert(err, IsNil) - - err = st.ReplaceIdentities(map[string]*state.Identity{ - "bob": nil, // nil means remove it - "mary": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 43}, - }, - "newguy": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 44}, - }, - }) - c.Assert(err, IsNil) - - // Ensure they were added/updated/deleted correctly. - identities := st.Identities() - c.Assert(identities, DeepEquals, map[string]*state.Identity{ - "mary": { - Name: "mary", - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 43}, - }, - "newguy": { - Name: "newguy", - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 44}, - }, - }) - - // Ensure validation is being done (full testing done in AddIdentity). - err = st.ReplaceIdentities(map[string]*state.Identity{ - "bill": { - Access: "admin", - }, - }) - c.Assert(err, ErrorMatches, `identity "bill" invalid: identity must have at least one type \("local", "basic", or "cert"\)`) - - // Ensure unique user ID testing is being done (full testing done in AddIdentity). - err = st.ReplaceIdentities(map[string]*state.Identity{ - "bob": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 43}, - }, - }) - c.Assert(err, ErrorMatches, `cannot have multiple identities with user ID 43 \(bob, mary\)`) -} - -func (s *identitiesSuite) TestRemoveIdentities(c *C) { - st := state.New(nil) - st.Lock() - defer st.Unlock() - - original := map[string]*state.Identity{ - "bill": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 43}, - }, - "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - "nancy": { - Access: state.MetricsAccess, - Basic: &state.BasicIdentity{Password: "hash"}, - }, - "queen": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1001}, - }, - } - err := st.AddIdentities(original) - c.Assert(err, IsNil) - - err = st.RemoveIdentities(map[string]struct{}{ - "bob": {}, - "mary": {}, - "nancy": {}, - }) - c.Assert(err, IsNil) - - // Ensure they were removed correctly. - identities := st.Identities() - c.Assert(identities, DeepEquals, map[string]*state.Identity{ - "bill": { - Name: "bill", - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 43}, - }, - "queen": { - Name: "queen", - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1001}, - }, - }) - - // Can't remove identity names that don't exist. - err = st.RemoveIdentities(map[string]struct{}{ - "bill": {}, - "bale": {}, - "mary": {}, - }) - c.Assert(err, ErrorMatches, "identities do not exist: bale, mary") -} - -func (s *identitiesSuite) TestIdentities(c *C) { - st := state.New(nil) - st.Lock() - defer st.Unlock() - - original := map[string]*state.Identity{ - "bob": { - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - "nancy": { - Access: state.MetricsAccess, - Basic: &state.BasicIdentity{Password: "hash"}, - }, - } - err := st.AddIdentities(original) - c.Assert(err, IsNil) - - // Ensure it returns correct results. - identities := st.Identities() - expected := map[string]*state.Identity{ - "bob": { - Name: "bob", - Access: state.ReadAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "mary": { - Name: "mary", - Access: state.AdminAccess, - Local: &state.LocalIdentity{UserID: 1000}, - }, - "nancy": { - Name: "nancy", - Access: state.MetricsAccess, - Basic: &state.BasicIdentity{Password: "hash"}, - }, - } - c.Assert(identities, DeepEquals, expected) - - // Ensure the map was cloned (mutations to first map won't affect second). - identities2 := st.Identities() - c.Assert(identities2, DeepEquals, expected) - identities["changed"] = &state.Identity{} - c.Assert(identities2, DeepEquals, expected) -} - -func (s *identitiesSuite) TestIdentityFromInputs(c *C) { - st := state.New(nil) - st.Lock() - defer st.Unlock() - - ids := map[string]*state.Identity{ - "uid": { - Access: state.MetricsAccess, - Local: &state.LocalIdentity{UserID: 42}, - }, - "basic": { - Access: state.ReadAccess, - Basic: &state.BasicIdentity{ - // password: test - Password: "$6$F9cFSVEKyO4gB1Wh$8S1BSKsNkF.jBAixGc4W7l80OpfCNk65LZBDHBng3NAmbcHuMj4RIm7992rrJ8YA.SJ0hvm.vGk2z483am4Ym1", - }, - }, - } - err := st.AddIdentities(ids) - c.Assert(err, IsNil) - - tests := []struct { - name string - userID *uint32 - basicUser string - basicPass string - expectedUser string - expectedAccess state.IdentityAccess - }{{ - name: "no inputs", - expectedUser: "", - }, { - // Basic authentication tests (medium priority) - name: "valid basic auth", - basicUser: "basic", - basicPass: "test", - expectedUser: "", // disabled in FIPS builds - }, { - name: "valid user invalid password", - basicUser: "basic", - basicPass: "wrong", - expectedUser: "", - }, { - name: "invalid user valid password", - basicUser: "nonexistent", - basicPass: "test", - expectedUser: "", - }, { - name: "invalid user invalid password", - basicUser: "nonexistent", - basicPass: "wrong", - expectedUser: "", - }, { - name: "empty user with password", - basicUser: "", - basicPass: "test", - expectedUser: "", - }, { - name: "user with empty password", - basicUser: "basic", - basicPass: "", - expectedUser: "", - }, { - name: "empty user and password", - basicUser: "", - basicPass: "", - expectedUser: "", - }, { - // Basic auth overrides UID - name: "basic auth with uid ignored", - basicUser: "basic", - basicPass: "test", - userID: ptr(uint32(42)), - expectedUser: "", // disabled in FIPS builds - }, { - name: "invalid basic auth with valid uid ignored", - basicUser: "basic", - basicPass: "wrong", - userID: ptr(uint32(42)), - expectedUser: "", - }, { - // Local/UID authentication tests (lowest priority) - name: "valid uid", - userID: ptr(uint32(42)), - expectedUser: "uid", - expectedAccess: state.MetricsAccess, - }, { - name: "invalid uid", - userID: ptr(uint32(100)), - expectedUser: "", - }, { - // Edge cases - name: "nil uid", - userID: nil, - expectedUser: "", - }} - - for _, test := range tests { - c.Logf("Running test: %s", test.name) - identity := st.IdentityFromInputs(test.userID, test.basicUser, test.basicPass) - - if test.expectedUser != "" { - c.Assert(identity, NotNil) - c.Assert(identity.Name, Equals, test.expectedUser) - c.Assert(identity.Access, Equals, test.expectedAccess) - } else { - c.Assert(identity, IsNil) - } - } -} - -func ptr[T any](v T) *T { - return &v -} diff --git a/internals/overlord/state/state.go b/internals/overlord/state/state.go index 65e2bd01f..8d02883f6 100644 --- a/internals/overlord/state/state.go +++ b/internals/overlord/state/state.go @@ -89,12 +89,13 @@ type State struct { // for registering runtime callbacks lastHandlerId int - backend Backend - data customData - changes map[string]*Change - tasks map[string]*Task - notices map[noticeKey]*Notice - identities map[string]*Identity + backend Backend + data customData + changes map[string]*Change + tasks map[string]*Task + notices map[noticeKey]*Notice + + legacyIdentities json.RawMessage noticeCond *sync.Cond latestWarningTime atomic.Pointer[time.Time] @@ -118,7 +119,6 @@ func New(backend Backend) *State { changes: make(map[string]*Change), tasks: make(map[string]*Task), notices: make(map[noticeKey]*Notice), - identities: make(map[string]*Identity), modified: true, cache: make(map[any]any), pendingChangeByAttr: make(map[string]func(*Change) bool), @@ -159,11 +159,14 @@ func (s *State) unlock() { } type marshalledState struct { - Data map[string]*json.RawMessage `json:"data"` - Changes map[string]*Change `json:"changes"` - Tasks map[string]*Task `json:"tasks"` - Notices []*Notice `json:"notices,omitempty"` - Identities map[string]*marshalledIdentity `json:"identities,omitempty"` + Data map[string]*json.RawMessage `json:"data"` + Changes map[string]*Change `json:"changes"` + Tasks map[string]*Task `json:"tasks"` + Notices []*Notice `json:"notices,omitempty"` + + // The "identities" key used to be stored directly on state at the top level, + // so be sure to read them from old state files. + LegacyIdentities json.RawMessage `json:"identities,omitempty"` LastChangeId int `json:"last-change-id"` LastTaskId int `json:"last-task-id"` @@ -171,36 +174,14 @@ type marshalledState struct { LastNoticeId int `json:"last-notice-id"` } -// marshalledIdentity is used specifically for marshalling to the state -// database file. Unlike apiIdentity, it should include secrets. -type marshalledIdentity struct { - Access string `json:"access"` - Local *marshalledLocalIdentity `json:"local,omitempty"` - Basic *marshalledBasicIdentity `json:"basic,omitempty"` - Cert *marshalledCertIdentity `json:"cert,omitempty"` -} - -type marshalledLocalIdentity struct { - UserID uint32 `json:"user-id"` -} - -type marshalledBasicIdentity struct { - Password string `json:"password"` -} - -type marshalledCertIdentity struct { - PEM string `json:"pem"` -} - // MarshalJSON makes State a json.Marshaller func (s *State) MarshalJSON() ([]byte, error) { s.reading() return json.Marshal(marshalledState{ - Data: s.data, - Changes: s.changes, - Tasks: s.tasks, - Notices: s.flattenNotices(nil), - Identities: s.marshalledIdentities(), + Data: s.data, + Changes: s.changes, + Tasks: s.tasks, + Notices: s.flattenNotices(nil), LastTaskId: s.lastTaskId, LastChangeId: s.lastChangeId, @@ -209,22 +190,6 @@ func (s *State) MarshalJSON() ([]byte, error) { }) } -func (s *State) marshalledIdentities() map[string]*marshalledIdentity { - marshalled := make(map[string]*marshalledIdentity, len(s.identities)) - for name, identity := range s.identities { - marshalled[name] = &marshalledIdentity{ - Access: string(identity.Access), - } - if identity.Local != nil { - marshalled[name].Local = &marshalledLocalIdentity{UserID: identity.Local.UserID} - } - if identity.Basic != nil { - marshalled[name].Basic = &marshalledBasicIdentity{Password: identity.Basic.Password} - } - } - return marshalled -} - // UnmarshalJSON makes State a json.Unmarshaller func (s *State) UnmarshalJSON(data []byte) error { s.writing() @@ -234,12 +199,11 @@ func (s *State) UnmarshalJSON(data []byte) error { return err } s.data = unmarshalled.Data + s.changes = unmarshalled.Changes s.tasks = unmarshalled.Tasks s.unflattenNotices(unmarshalled.Notices) - if err := s.unmarshalIdentities(unmarshalled.Identities); err != nil { - return err - } + s.legacyIdentities = unmarshalled.LegacyIdentities s.lastChangeId = unmarshalled.LastChangeId s.lastTaskId = unmarshalled.LastTaskId s.lastLaneId = unmarshalled.LastLaneId @@ -255,21 +219,10 @@ func (s *State) UnmarshalJSON(data []byte) error { return nil } -func (s *State) unmarshalIdentities(marshalled map[string]*marshalledIdentity) error { - s.identities = make(map[string]*Identity, len(marshalled)) - for name, mi := range marshalled { - s.identities[name] = &Identity{ - Name: name, - Access: IdentityAccess(mi.Access), - } - if mi.Local != nil { - s.identities[name].Local = &LocalIdentity{UserID: mi.Local.UserID} - } - if mi.Basic != nil { - s.identities[name].Basic = &BasicIdentity{Password: mi.Basic.Password} - } - } - return nil +// LegacyIdentities is exported for use in patch2.go to perform the migration. +func (s *State) LegacyIdentities() json.RawMessage { + s.reading() + return s.legacyIdentities } func (s *State) checkpointData() []byte { diff --git a/internals/overlord/state/state_test.go b/internals/overlord/state/state_test.go index 1153b1911..47305c921 100644 --- a/internals/overlord/state/state_test.go +++ b/internals/overlord/state/state_test.go @@ -607,7 +607,6 @@ func (ss *stateSuite) TestEmptyStateDataAndCheckpointReadAndSet(c *C) { "changes", "tasks", "notices", - "identities", "cache", "pendingChangeByAttr", "taskHandlers", diff --git a/staticcheck.conf b/staticcheck.conf deleted file mode 100644 index 9f992fc0a..000000000 --- a/staticcheck.conf +++ /dev/null @@ -1,5 +0,0 @@ -checks = [ - "inherit", - "-SA5008", # duplicate struct tag (cli/cmd_*.go use this for "choice") - "-SA1012", # do not pass a nil Context (tomb.Context specifically allows it) -]