Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check file exists before using bolt.Open #146

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

spurin
Copy link
Contributor

@spurin spurin commented Nov 4, 2024

Hi All,

Thank you for your dedicated work on this wonderful project. I was in the process of building something similar and was delighted to come across your existing efforts.

I wanted to address an issue I encountered while using auger with the -f extract option when specifying an etcd snapshot that does not exist.

As many of you may know, this utility uses the bolt "go.etcd.io/bbolt" library.

When opening files, it will automatically create a new file if it does not exist. This behaviour is depicted in the following example from the Bolt repository:

func main() {
	// Open the my.db data file in your current directory.
	// It will be created if it doesn't exist.
	db, err := bolt.Open("my.db", 0600, nil)
	if err != nil {
		log.Fatal(err)
	}
	defer db.Close()

	...
}

Although this functionality is useful in some contexts, it becomes problematic for us when attempting to access an existing snapshot. Specifically, we do not want to inadvertently create a zero-length file if an incorrect filename is provided.

This pull request introduces a straightforward fix by adding a preliminary OS check before executing bolt.Open. It also improves user feedback by clarifying error messages when files do not exist. An example with this change implemented:

# auger extract -f /file_doesnt_exist
Error: file does not exist: /file_doesnt_exist
Usage:
  auger extract [flags]

Examples:

        # Find an etcd value by it's key and extract it from a boltdb file:
        auger extract -f <boltdb-file> -k /registry/pods/default/<pod-name>

        # List the keys and size of all entries in etcd
        auger extract -f <boltdb-file> --fields=key,value-size

        # Extract a specific field from each kubernetes object
        auger extract -f <boltdb-file> --template="{{.Value.metadata.creationTimestamp}}"

        # Extract kubernetes objects using a filter
        auger extract -f <boltdb-file> --filter=".Value.metadata.namespace=kube-system"

        # Extract the etcd value stored in page 10, item 0 of a boltdb file:
        bolt page --item 0 --value-only <boltdb-file> 10 | auger extract --leaf-item

        # Extract the etcd key stored in page 10, item 0 of a boltdb file:
        bolt page --item 0 --value-only <boltdb-file> 10 | auger extract --leaf-item --print-key


Flags:
      --fields string           Fields to include when listing entries, comma separated list of: [key value-size all-versions-value-size version-count value] (default "key")
  -f, --file string             Bolt DB '.db' filename
      --filter string           Filter entries using a comma separated list of '<field>=value' constraints. Fields used in filters use the same naming as --template fields, e.g. .Value.metadata.namespace
  -h, --help                    help for extract
  -k, --key string              Etcd object key to find in boltdb file
      --keys-by-prefix string   List out all keys with the given prefix
      --leaf-item               Read the input as a boltdb leaf page item.
      --list-versions           List out all versions of the key, requires --key
      --meta-summary            Print a summary of the metadata of the matching entry
  -o, --output string           Output format. One of: json|yaml|proto (default "yaml")
      --print-key               Print the key of the matching entry
      --raw                     Don't attempt to decode the etcd value
  -r, --revision int            etcd revision to retrieve data at, if 0, the latest revision is used, defaults to 0
      --template string         golang template to use when listing entries, see https://golang.org/pkg/text/template, template is provided an object with the fields: Key, Version, Value, TypeMeta, Stats. The Value field contains the entire kubernetes resource object which also may be dereferenced using a dot seperated path.
  -v, --version string          Version of etcd key to find, defaults to latest version

file does not exist: /file_doesnt_exist

As expected when the snapshot file exists auger works as expected -

# auger extract -f /tmp/etcd.dump | more
/registry/apiextensions.k8s.io/customresourcedefinitions/bgpconfigurations.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/bgppeers.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/blockaffinities.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/caliconodestatuses.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/clusterinformations.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/felixconfigurations.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/globalnetworkpolicies.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/globalnetworksets.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/hostendpoints.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/ipamblocks.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/ipamconfigs.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/ipamhandles.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/ippools.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/ipreservations.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/kubecontrollersconfigurations.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/networkpolicies.crd.projectcalico.org
/registry/apiextensions.k8s.io/customresourcedefinitions/networksets.crd.projectcalico.org

I believe this improvement will prevent any unintended file creation and provide clearer guidance when files are missing.

Hope this helps and looking forward to your feedback on this proposal.

@spurin spurin force-pushed the dont_create_snapshot_if_doesnt_exist branch from c58d99e to fa96b35 Compare November 4, 2024 18:51
Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Great suggestion, many thanks @spurin 👍🏻

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jmhbnz, spurin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jmhbnz jmhbnz merged commit 54e202a into etcd-io:main Nov 4, 2024
3 checks passed
@spurin spurin deleted the dont_create_snapshot_if_doesnt_exist branch November 5, 2024 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants