From 7f78c09db61b693185ab869806f45f57cc81bfec 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 cmd/run/main_test.go | 16 ++-- .../testdata/validchaincode/chaincode.json | 2 +- 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 ++++++++++++------- 17 files changed, 162 insertions(+), 70 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/cmd/run/main_test.go b/cmd/run/main_test.go index 86a8cf4..9888514 100644 --- a/cmd/run/main_test.go +++ b/cmd/run/main_test.go @@ -95,10 +95,10 @@ var _ = Describe("Main", func() { ).Should(gbytes.Say(`run \[\d+\] DEBUG: FABRIC_K8S_BUILDER_SERVICE_ACCOUNT=chaincode`)) Eventually( session.Err, - ).Should(gbytes.Say(`run \[\d+\]: Running chaincode ID CHAINCODE_LABEL:CHAINCODE_HASH in kubernetes pod chaincode/hlfcc-chaincodelabel-f15ukm9v906aq`)) + ).Should(gbytes.Say(`run \[\d+\]: Running chaincode ID CHAINCODE_LABEL:6f98c4bb29414771312eddd1a813eef583df2121c235c4797792f141a46d4b45 in kubernetes pod chaincode/hlfcc-chaincodelabel-f887209uhojj2`)) pipe := script.Exec( - "kubectl wait --for=condition=ready pod --timeout=120s --namespace=chaincode -l fabric-builder-k8s-peerid=core-peer-id-abcdefghijklmnopqrstuvwxyz-0123456789", + "kubectl wait --for=condition=ready pod --timeout=120s --namespace=chaincode -l fabric-builder-k8s-cclabel=CHAINCODE_LABEL", ) _, err = pipe.Stdout() Expect(err).NotTo(HaveOccurred()) @@ -109,7 +109,7 @@ var _ = Describe("Main", func() { "pod", "--namespace=chaincode", "-l", - "fabric-builder-k8s-peerid=core-peer-id-abcdefghijklmnopqrstuvwxyz-0123456789", + "fabric-builder-k8s-cclabel=CHAINCODE_LABEL", } descCommand := exec.Command("kubectl", descArgs...) descSession, err := gexec.Start(descCommand, GinkgoWriter, GinkgoWriter) @@ -117,11 +117,15 @@ var _ = Describe("Main", func() { Eventually(descSession).Should(gexec.Exit(0)) Eventually(descSession.Out).Should(gbytes.Say(`Namespace:\s+chaincode`)) - Eventually(descSession.Out).Should(gbytes.Say(`fabric-builder-k8s-mspid=MSPID`)) Eventually( descSession.Out, - ).Should(gbytes.Say(`fabric-builder-k8s-ccid:\s+CHAINCODE_LABEL:CHAINCODE_HASH`)) - Eventually(descSession.Out).Should(gbytes.Say(`CORE_CHAINCODE_ID_NAME:\s+CHAINCODE_LABEL:CHAINCODE_HASH`)) + ).Should(gbytes.Say(`fabric-builder-k8s-ccid:\s+CHAINCODE_LABEL:6f98c4bb29414771312eddd1a813eef583df2121c235c4797792f141a46d4b45`)) + Eventually(descSession.Out).Should(gbytes.Say(`fabric-builder-k8s-mspid:\s+MSPID`)) + Eventually(descSession.Out).Should(gbytes.Say(`fabric-builder-k8s-peeraddress:\s+PEER_ADDRESS`)) + Eventually(descSession.Out).Should(gbytes.Say(`fabric-builder-k8s-peerid:\s+core-peer-id-abcdefghijklmnopqrstuvwxyz-0123456789`)) + Eventually( + descSession.Out, + ).Should(gbytes.Say(`CORE_CHAINCODE_ID_NAME:\s+CHAINCODE_LABEL:6f98c4bb29414771312eddd1a813eef583df2121c235c4797792f141a46d4b45`)) Eventually(descSession.Out).Should(gbytes.Say(`CORE_PEER_ADDRESS:\s+PEER_ADDRESS`)) Eventually(descSession.Out).Should(gbytes.Say(`CORE_PEER_LOCALMSPID:\s+MSPID`)) }, diff --git a/cmd/run/testdata/validchaincode/chaincode.json b/cmd/run/testdata/validchaincode/chaincode.json index 53ffb72..764268b 100644 --- a/cmd/run/testdata/validchaincode/chaincode.json +++ b/cmd/run/testdata/validchaincode/chaincode.json @@ -1,5 +1,5 @@ { - "chaincode_id": "CHAINCODE_LABEL:CHAINCODE_HASH", + "chaincode_id": "CHAINCODE_LABEL:6f98c4bb29414771312eddd1a813eef583df2121c235c4797792f141a46d4b45", "peer_address": "PEER_ADDRESS", "client_cert": "CLIENT_CERT", "client_key": "CLIENT_KEY", 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",