From 04f5b5d12ff30645bb7fce50df5b70a35c3d1960 Mon Sep 17 00:00:00 2001 From: James Taylor Date: Fri, 24 May 2024 11:03:56 +0000 Subject: [PATCH] Make sure Kubernetes object labels are valid Base32 encode the package hash and validate the chaincode label in the build phase Move labels that cannot be guaranteed to be valid to annotations instead Closes #89 Signed-off-by: James Taylor --- cmd/build/main_test.go | 22 +++-- .../ccmetadata/invalidlabel/metadata.json | 4 + .../invalidlabellength/metadata.json | 4 + .../ccmetadata/invalidmetadata/metadata.json | 1 + .../ccmetadata/validmetadata/metadata.json | 4 + .../{ => ccsrc}/invalidimage/image.json | 0 .../{ => ccsrc}/validimage/image.json | 0 .../withmetadata/META-INF/test/test.txt | 0 .../{ => ccsrc}/withmetadata/image.json | 0 docs/concepts/chaincode-builder.md | 16 ++-- docs/concepts/chaincode-package.md | 2 + internal/builder/build.go | 17 +++- internal/builder/detect.go | 23 +---- internal/util/files.go | 31 +++++++ internal/util/k8s.go | 90 ++++++++++++------- 15 files changed, 151 insertions(+), 63 deletions(-) create mode 100644 cmd/build/testdata/ccmetadata/invalidlabel/metadata.json create mode 100644 cmd/build/testdata/ccmetadata/invalidlabellength/metadata.json create mode 100644 cmd/build/testdata/ccmetadata/invalidmetadata/metadata.json create mode 100644 cmd/build/testdata/ccmetadata/validmetadata/metadata.json rename cmd/build/testdata/{ => ccsrc}/invalidimage/image.json (100%) rename cmd/build/testdata/{ => ccsrc}/validimage/image.json (100%) rename cmd/build/testdata/{ => ccsrc}/withmetadata/META-INF/test/test.txt (100%) rename cmd/build/testdata/{ => ccsrc}/withmetadata/image.json (100%) diff --git a/cmd/build/main_test.go b/cmd/build/main_test.go index 7b820f0..d1f391f 100644 --- a/cmd/build/main_test.go +++ b/cmd/build/main_test.go @@ -24,14 +24,26 @@ var _ = Describe("Main", func() { Eventually(session).Should(gexec.Exit(expectedErrorCode)) }, - Entry("When the image.json file exists", 0, func() []string { - return []string{"./testdata/validimage", "CHAINCODE_METADATA_DIR", tempDir} + Entry("When the image.json and metadata.json files exist", 0, func() []string { + return []string{"./testdata/ccsrc/validimage", "./testdata/ccmetadata/validmetadata", tempDir} }), Entry("When the image.json file does not exist", 1, func() []string { - return []string{"CHAINCODE_SOURCE_DIR", "CHAINCODE_METADATA_DIR", "BUILD_OUTPUT_DIR"} + return []string{"CHAINCODE_SOURCE_DIR", "./testdata/ccmetadata/validmetadata", "BUILD_OUTPUT_DIR"} }), Entry("When the image.json file is invalid", 1, func() []string { - return []string{"./testdata/invalidimage", "CHAINCODE_METADATA_DIR", tempDir} + return []string{"./testdata/invalidimage", "./testdata/ccmetadata/validmetadata", "BUILD_OUTPUT_DIR"} + }), + Entry("When the metadata.json file does not exist", 1, func() []string { + return []string{"./testdata/validimage", "CHAINCODE_METADATA_DIR", "BUILD_OUTPUT_DIR"} + }), + Entry("When the metadata.json file is invalid", 1, func() []string { + return []string{"./testdata/validimage", "./testdata/ccmetadata/invalidmetadata", "BUILD_OUTPUT_DIR"} + }), + Entry("When the metadata.json contains an invalid label", 1, func() []string { + return []string{"./testdata/validimage", "./testdata/ccmetadata/invalidlabel", "BUILD_OUTPUT_DIR"} + }), + Entry("When the metadata.json contains an invalid label length", 1, func() []string { + return []string{"./testdata/validimage", "./testdata/ccmetadata/invalidlabellength", "BUILD_OUTPUT_DIR"} }), Entry("When too few arguments are provided", 1, func() []string { return []string{"CHAINCODE_SOURCE_DIR"} @@ -47,7 +59,7 @@ var _ = Describe("Main", func() { ) It("should copy chaincode metadata to the build output directory", func() { - args := []string{"./testdata/withmetadata", "CHAINCODE_METADATA_DIR", tempDir} + args := []string{"./testdata/ccsrc/withmetadata", "./testdata/ccmetadata/validmetadata", tempDir} command := exec.Command(buildCmdPath, args...) session, err := gexec.Start(command, GinkgoWriter, GinkgoWriter) Expect(err).NotTo(HaveOccurred()) diff --git a/cmd/build/testdata/ccmetadata/invalidlabel/metadata.json b/cmd/build/testdata/ccmetadata/invalidlabel/metadata.json new file mode 100644 index 0000000..7781b45 --- /dev/null +++ b/cmd/build/testdata/ccmetadata/invalidlabel/metadata.json @@ -0,0 +1,4 @@ +{ + "type": "k8s", + "label": "42" +} diff --git a/cmd/build/testdata/ccmetadata/invalidlabellength/metadata.json b/cmd/build/testdata/ccmetadata/invalidlabellength/metadata.json new file mode 100644 index 0000000..0d0ac6e --- /dev/null +++ b/cmd/build/testdata/ccmetadata/invalidlabellength/metadata.json @@ -0,0 +1,4 @@ +{ + "type": "k8s", + "label": "fabfabfabfabcarfabfabfabfabcarfabfabfabfabcarfabfabfabfabcarfabfabfabfabcarfabfabfabfabcarfabfabfabfabcarfabfabfabfabcar" +} diff --git a/cmd/build/testdata/ccmetadata/invalidmetadata/metadata.json b/cmd/build/testdata/ccmetadata/invalidmetadata/metadata.json new file mode 100644 index 0000000..cde36bd --- /dev/null +++ b/cmd/build/testdata/ccmetadata/invalidmetadata/metadata.json @@ -0,0 +1 @@ +type=k8s diff --git a/cmd/build/testdata/ccmetadata/validmetadata/metadata.json b/cmd/build/testdata/ccmetadata/validmetadata/metadata.json new file mode 100644 index 0000000..1e307f1 --- /dev/null +++ b/cmd/build/testdata/ccmetadata/validmetadata/metadata.json @@ -0,0 +1,4 @@ +{ + "type": "k8s", + "label": "basic" +} diff --git a/cmd/build/testdata/invalidimage/image.json b/cmd/build/testdata/ccsrc/invalidimage/image.json similarity index 100% rename from cmd/build/testdata/invalidimage/image.json rename to cmd/build/testdata/ccsrc/invalidimage/image.json diff --git a/cmd/build/testdata/validimage/image.json b/cmd/build/testdata/ccsrc/validimage/image.json similarity index 100% rename from cmd/build/testdata/validimage/image.json rename to cmd/build/testdata/ccsrc/validimage/image.json diff --git a/cmd/build/testdata/withmetadata/META-INF/test/test.txt b/cmd/build/testdata/ccsrc/withmetadata/META-INF/test/test.txt similarity index 100% rename from cmd/build/testdata/withmetadata/META-INF/test/test.txt rename to cmd/build/testdata/ccsrc/withmetadata/META-INF/test/test.txt diff --git a/cmd/build/testdata/withmetadata/image.json b/cmd/build/testdata/ccsrc/withmetadata/image.json similarity index 100% rename from cmd/build/testdata/withmetadata/image.json rename to cmd/build/testdata/ccsrc/withmetadata/image.json diff --git a/docs/concepts/chaincode-builder.md b/docs/concepts/chaincode-builder.md index f243202..90dd825 100644 --- a/docs/concepts/chaincode-builder.md +++ b/docs/concepts/chaincode-builder.md @@ -4,11 +4,15 @@ From version 2.0, Hyperledger Fabric supports External Builders and Launchers to External Builders and Launchers are a collection of executables — `detect`, `build`, `release` and `run` — that the peer calls in order to build, launch, and discover chaincode. The k8s builder executables do the following. -| Executable | Description | -| ----------- | ----------------------------------------------------------------------------------------------------------------- | -| detect | Detects chaincode packages with a type of `k8s` | -| build | No-op (a chaincode image must be built and published to a container registry before it can be deployed to Fabric) | -| release | No-op | -| run | Starts a Kubernetes pod using the chaincode image identified by an immutable digest | +| Executable | Description | +| ----------- | ----------------------------------------------------------------------------------- | +| detect | Detects chaincode packages with a type of `k8s` | +| build | Checks that the chaincode label is a valid Kubernetes object label[^1] | +| release | No-op | +| run | Starts a Kubernetes pod using the chaincode image identified by an immutable digest | + +[^1]: + The k8s builder *does not* build the chaincode image. + A chaincode image must be built and published to a container registry before it can be deployed to Fabric using the k8s builder. For more information about Fabric builders, see the [External Builders and Launchers](https://hyperledger-fabric.readthedocs.io/en/latest/cc_launcher.html) documentation. diff --git a/docs/concepts/chaincode-package.md b/docs/concepts/chaincode-package.md index dadf456..7a9e9cc 100644 --- a/docs/concepts/chaincode-package.md +++ b/docs/concepts/chaincode-package.md @@ -24,6 +24,8 @@ The k8s builder will detect chaincode packages which have a type of `k8s`. For e } ``` +The k8s builder uses the chaincode label to label Kubernetes objects, so it must be a [valid Kubernetes label value](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set). + ## code.tar.gz Unlike other chaincode packages, the source artifacts in a k8s chaincode package do not contain the chaincode source files. diff --git a/internal/builder/build.go b/internal/builder/build.go index e1b8dbb..94ae4a5 100644 --- a/internal/builder/build.go +++ b/internal/builder/build.go @@ -4,9 +4,11 @@ package builder import ( "context" + "fmt" "github.com/hyperledger-labs/fabric-builder-k8s/internal/log" "github.com/hyperledger-labs/fabric-builder-k8s/internal/util" + "k8s.io/apimachinery/pkg/util/validation" ) type Build struct { @@ -19,7 +21,20 @@ func (b *Build) Run(ctx context.Context) error { logger := log.New(ctx) logger.Debugln("Building chaincode...") - err := util.CopyImageJSON(logger, b.ChaincodeSourceDirectory, b.BuildOutputDirectory) + metadata, err := util.ReadMetadataJSON(logger, b.ChaincodeMetadataDirectory) + if err != nil { + return err + } + + if errs := validation.IsDNS1035Label(metadata.Label); len(errs) != 0 { + return fmt.Errorf( + "chaincode label '%s' must be a valid RFC1035 label: %v", + metadata.Label, + errs, + ) + } + + err = util.CopyImageJSON(logger, b.ChaincodeSourceDirectory, b.BuildOutputDirectory) if err != nil { return err } diff --git a/internal/builder/detect.go b/internal/builder/detect.go index 658f818..5fba369 100644 --- a/internal/builder/detect.go +++ b/internal/builder/detect.go @@ -4,14 +4,11 @@ package builder import ( "context" - "encoding/json" "errors" - "fmt" - "os" - "path/filepath" "strings" "github.com/hyperledger-labs/fabric-builder-k8s/internal/log" + "github.com/hyperledger-labs/fabric-builder-k8s/internal/util" ) type Detect struct { @@ -19,29 +16,15 @@ type Detect struct { ChaincodeMetadataDirectory string } -type metadata struct { - Label string `json:"label"` - Type string `json:"type"` -} - var ErrUnsupportedChaincodeType = errors.New("chaincode type not supported") func (d *Detect) Run(ctx context.Context) error { logger := log.New(ctx) logger.Debugln("Checking chaincode type...") - mdpath := filepath.Join(d.ChaincodeMetadataDirectory, "metadata.json") - - mdbytes, err := os.ReadFile(mdpath) - if err != nil { - return fmt.Errorf("unable to read %s: %w", mdpath, err) - } - - var metadata metadata - - err = json.Unmarshal(mdbytes, &metadata) + metadata, err := util.ReadMetadataJSON(logger, d.ChaincodeMetadataDirectory) if err != nil { - return fmt.Errorf("unable to process %s: %w", mdpath, err) + return err } if strings.ToLower(metadata.Type) == "k8s" { diff --git a/internal/util/files.go b/internal/util/files.go index f85ad0a..7409eb7 100644 --- a/internal/util/files.go +++ b/internal/util/files.go @@ -28,9 +28,16 @@ type ImageJSON struct { Digest string `json:"digest"` } +// MetadataJSON represents the metadata.json file in the k8s chaincode package. +type MetadataJSON struct { + Label string `json:"label"` + Type string `json:"type"` +} + const ( ChaincodeFile = "chaincode.json" ImageFile = "image.json" + MetadataFile = "metadata.json" MetadataDir = "META-INF" ) @@ -77,3 +84,27 @@ func ReadImageJSON(logger *log.CmdLogger, dir string) (*ImageJSON, error) { return &imageData, nil } + +// ReadMetadataJSON reads and parses the metadata.json file in the provided directory. +func ReadMetadataJSON(logger *log.CmdLogger, dir string) (*MetadataJSON, error) { + metadataJSONPath := filepath.Join(dir, MetadataFile) + logger.Debugf("Reading %s...", metadataJSONPath) + + metadataJSONContents, err := os.ReadFile(metadataJSONPath) + if err != nil { + return nil, fmt.Errorf("unable to read %s: %w", metadataJSONPath, err) + } + + var metadata MetadataJSON + if err := json.Unmarshal(metadataJSONContents, &metadata); err != nil { + return nil, fmt.Errorf("unable to parse %s: %w", metadataJSONPath, err) + } + + logger.Debugf("Label: %s\nType: %s\n", metadata.Label, metadata.Type) + + if len(metadata.Label) == 0 || len(metadata.Type) == 0 { + return nil, fmt.Errorf("%s file must contain 'label' and 'type'", metadataJSONPath) + } + + return &metadata, nil +} diff --git a/internal/util/k8s.go b/internal/util/k8s.go index 4aa2593..cc860f5 100644 --- a/internal/util/k8s.go +++ b/internal/util/k8s.go @@ -6,6 +6,7 @@ import ( "context" "encoding/base32" "encoding/base64" + "encoding/hex" "fmt" "hash/fnv" "os" @@ -226,28 +227,55 @@ func GetKubeNamespace() (string, error) { return string(namespace), nil } +func getLabels(chaincodeData *ChaincodeJSON) (map[string]string, error) { + packageID := NewChaincodePackageID(chaincodeData.ChaincodeID) + + packageHashBytes, err := hex.DecodeString(packageID.Hash) + if err != nil { + return nil, fmt.Errorf("error decoding chaincode package hash %s: %w", packageID.Hash, err) + } + + encodedPackageHash := base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(packageHashBytes) + + return map[string]string{ + "app.kubernetes.io/name": "hyperledger-fabric", + "app.kubernetes.io/component": "chaincode", + "app.kubernetes.io/created-by": "fabric-builder-k8s", + "app.kubernetes.io/managed-by": "fabric-builder-k8s", + "fabric-builder-k8s-cclabel": packageID.Label, + "fabric-builder-k8s-cchash": encodedPackageHash, + }, nil +} + +func getAnnotations(peerID string, chaincodeData *ChaincodeJSON) map[string]string { + return map[string]string{ + "fabric-builder-k8s-ccid": chaincodeData.ChaincodeID, + "fabric-builder-k8s-mspid": chaincodeData.MspID, + "fabric-builder-k8s-peeraddress": chaincodeData.PeerAddress, + "fabric-builder-k8s-peerid": peerID, + } +} + func getChaincodePodObject( imageData *ImageJSON, namespace, serviceAccount, podName, peerID string, chaincodeData *ChaincodeJSON, -) *apiv1.Pod { +) (*apiv1.Pod, error) { chaincodeImage := imageData.Name + "@" + imageData.Digest + labels, err := getLabels(chaincodeData) + if err != nil { + return nil, fmt.Errorf("error getting chaincode job labels for chaincode ID %s: %w", chaincodeData.ChaincodeID, err) + } + + annotations := getAnnotations(peerID, chaincodeData) + return &apiv1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: podName, - Namespace: namespace, - Labels: map[string]string{ - "app.kubernetes.io/name": "fabric", - "app.kubernetes.io/component": "chaincode", - "app.kubernetes.io/created-by": "fabric-builder-k8s", - "app.kubernetes.io/managed-by": "fabric-builder-k8s", - "fabric-builder-k8s-mspid": chaincodeData.MspID, - "fabric-builder-k8s-peerid": peerID, - }, - Annotations: map[string]string{ - "fabric-builder-k8s-ccid": chaincodeData.ChaincodeID, - }, + Name: podName, + Namespace: namespace, + Labels: labels, + Annotations: annotations, }, Spec: apiv1.PodSpec{ ServiceAccountName: serviceAccount, @@ -314,17 +342,20 @@ func getChaincodePodObject( }, }, }, - } + }, nil } func getChaincodeSecretApplyConfiguration( secretName, namespace, peerID string, chaincodeData *ChaincodeJSON, -) *applycorev1.SecretApplyConfiguration { - annotations := map[string]string{ - "fabric-builder-k8s-ccid": chaincodeData.ChaincodeID, +) (*applycorev1.SecretApplyConfiguration, error) { + labels, err := getLabels(chaincodeData) + if err != nil { + return nil, fmt.Errorf("error getting chaincode job labels for chaincode ID %s: %w", chaincodeData.ChaincodeID, err) } + annotations := getAnnotations(peerID, chaincodeData) + data := map[string]string{ "peer.crt": chaincodeData.RootCert, "client_pem.crt": chaincodeData.ClientCert, @@ -333,21 +364,12 @@ func getChaincodeSecretApplyConfiguration( "client.key": base64.StdEncoding.EncodeToString([]byte(chaincodeData.ClientKey)), } - labels := map[string]string{ - "app.kubernetes.io/name": "fabric", - "app.kubernetes.io/component": "chaincode", - "app.kubernetes.io/created-by": "fabric-builder-k8s", - "app.kubernetes.io/managed-by": "fabric-builder-k8s", - "fabric-builder-k8s-mspid": chaincodeData.MspID, - "fabric-builder-k8s-peerid": peerID, - } - return applycorev1. Secret(secretName, namespace). WithAnnotations(annotations). WithLabels(labels). WithStringData(data). - WithType(apiv1.SecretTypeOpaque) + WithType(apiv1.SecretTypeOpaque), nil } func ApplyChaincodeSecrets( @@ -357,7 +379,10 @@ func ApplyChaincodeSecrets( secretName, namespace, peerID string, chaincodeData *ChaincodeJSON, ) error { - secret := getChaincodeSecretApplyConfiguration(secretName, namespace, peerID, chaincodeData) + secret, err := getChaincodeSecretApplyConfiguration(secretName, namespace, peerID, chaincodeData) + if err != nil { + return fmt.Errorf("error getting chaincode secret definition for chaincode ID %s: %w", chaincodeData.ChaincodeID, err) + } result, err := secretsClient.Apply( ctx, @@ -438,7 +463,7 @@ func CreateChaincodePod( chaincodeData *ChaincodeJSON, imageData *ImageJSON, ) (*apiv1.Pod, error) { - podDefinition := getChaincodePodObject( + podDefinition, err := getChaincodePodObject( imageData, namespace, serviceAccount, @@ -446,8 +471,11 @@ func CreateChaincodePod( peerID, chaincodeData, ) + if err != nil { + return nil, fmt.Errorf("error getting chaincode pod definition for chaincode ID %s: %w", chaincodeData.ChaincodeID, err) + } - err := deleteChaincodePod(ctx, logger, podsClient, objectName, namespace, chaincodeData) + err = deleteChaincodePod(ctx, logger, podsClient, objectName, namespace, chaincodeData) if err != nil { return nil, fmt.Errorf( "unable to delete existing chaincode pod %s/%s for chaincode ID %s: %w",