fix(feature): resolve all lint violations in pkg/devcontainer/feature

pkg/devcontainer/build.go

@@ -71,14 +71,14 @@ func (r *runner) extendImage(
 	}
 
 	// get extend image build info
-	extendedBuildInfo, err := feature.GetExtendedBuildInfo(
-		substitutionContext,
-		imageBuildInfo,
-		imageBase,
-		parsedConfig,
-		options.ForceBuild,
-		featureSecretOpts(options),
-	)
+	extendedBuildInfo, err := feature.GetExtendedBuildInfo(&feature.ExtendedBuildInfoOptions{
+		Ctx:                substitutionContext,
+		ImageBuildInfo:     imageBuildInfo,
+		Target:             imageBase,
+		DevContainerConfig: parsedConfig,
+		ForceBuild:         options.ForceBuild,
+		SecretOpts:         featureSecretOpts(options),
+	})
 	if err != nil {
 		return nil, fmt.Errorf("get extended build info: %w", err)
 	}
@@ -153,14 +153,14 @@ func (r *runner) buildAndExtendImage(
 	}
 
 	// get extend image build info
-	extendedBuildInfo, err := feature.GetExtendedBuildInfo(
-		substitutionContext,
-		imageBuildInfo,
-		imageBase,
-		parsedConfig,
-		options.ForceBuild,
-		featureSecretOpts(options),
-	)
+	extendedBuildInfo, err := feature.GetExtendedBuildInfo(&feature.ExtendedBuildInfoOptions{
+		Ctx:                substitutionContext,
+		ImageBuildInfo:     imageBuildInfo,
+		Target:             imageBase,
+		DevContainerConfig: parsedConfig,
+		ForceBuild:         options.ForceBuild,
+		SecretOpts:         featureSecretOpts(options),
+	})
 	if err != nil {
 		return nil, fmt.Errorf("get extended build info: %w", err)
 	}

pkg/devcontainer/compose.go

@@ -685,14 +685,14 @@ func (r *runner) buildAndExtendDockerCompose(
 	if featureSecretsFile != "" {
 		secretOpts = &feature.SecretOptions{SecretsFile: featureSecretsFile}
 	}
-	extendImageBuildInfo, err := feature.GetExtendedBuildInfo(
-		substitutionContext,
-		imageBuildInfo,
-		buildTarget,
-		parsedConfig,
-		false,
-		secretOpts,
-	)
+	extendImageBuildInfo, err := feature.GetExtendedBuildInfo(&feature.ExtendedBuildInfoOptions{
+		Ctx:                substitutionContext,
+		ImageBuildInfo:     imageBuildInfo,
+		Target:             buildTarget,
+		DevContainerConfig: parsedConfig,
+		ForceBuild:         false,
+		SecretOpts:         secretOpts,
+	})
 	if err != nil {
 		return composeExtendResult{}, err
 	}

pkg/devcontainer/feature/annotations_test.go

@@ -13,10 +13,10 @@ import (
 func TestSaveAnnotations(t *testing.T) {
 	dir := t.TempDir()
 	annotations := map[string]string{
-		"org.opencontainers.image.title":       "Go",
-		"org.opencontainers.image.description": "Installs Go and common Go tools",
-		"org.opencontainers.image.version":     "1.2.3",
-		"org.opencontainers.image.source":      "https://github.com/devcontainers/features",
+		AnnotationTitle:                   "Go",
+		AnnotationDescription:             "Installs Go and common Go tools",
+		AnnotationVersion:                 "1.2.3",
+		"org.opencontainers.image.source": "https://github.com/devcontainers/features",
 	}
 
 	saveAnnotations(dir, annotations)
@@ -47,8 +47,8 @@ func TestLoadOCIAnnotations_Present(t *testing.T) {
 	require.NoError(t, os.MkdirAll(extractedDir, 0o750))
 
 	annotations := map[string]string{
-		"org.opencontainers.image.title":         "Node.js",
-		"org.opencontainers.image.description":   "Installs Node.js and common npm tools",
+		AnnotationTitle:                          "Node.js",
+		AnnotationDescription:                    "Installs Node.js and common npm tools",
 		"org.opencontainers.image.authors":       "Dev Containers",
 		"org.opencontainers.image.url":           "https://github.com/devcontainers/features/tree/main/src/node",
 		"org.opencontainers.image.documentation": "https://containers.dev/features",
@@ -94,9 +94,9 @@ func TestLogOCIAnnotations_NoTitle(t *testing.T) {
 
 func TestLogOCIAnnotations_WithTitle(t *testing.T) {
 	annotations := map[string]string{
-		"org.opencontainers.image.title":       "Go",
-		"org.opencontainers.image.description": "Installs Go",
-		"org.opencontainers.image.version":     "1.0.0",
+		AnnotationTitle:       "Go",
+		AnnotationDescription: "Installs Go",
+		AnnotationVersion:     testVersion1_0_0,
 	}
 	// Should not panic
 	logOCIAnnotations("ghcr.io/devcontainers/features/go:1", annotations)

pkg/devcontainer/feature/collection_test.go

@@ -17,7 +17,10 @@ import (
 	"github.com/stretchr/testify/suite"
 )
 
-const testFeatureNode = "node"
+const (
+	testFeatureNode  = "node"
+	testVersion1_0_0 = "1.0.0"
+)
 
 type CollectionTestSuite struct {
 	suite.Suite
@@ -103,7 +106,7 @@ func (s *CollectionTestSuite) TestFetchCollection_InvalidJSON() {
 func (s *CollectionTestSuite) TestListCollectionFeatures() {
 	collection := Collection{
 		Features: []CollectionFeature{
-			{ID: "rust", Version: "1.0.0", Name: "Rust"},
+			{ID: "rust", Version: testVersion1_0_0, Name: "Rust"},
 			{ID: "python", Version: "3.0.0", Name: "Python"},
 			{ID: "java", Version: "1.5.0", Name: "Java"},
 		},
@@ -139,7 +142,7 @@ func (s *CollectionTestSuite) TestFetchCollection_DeprecatedFeature() {
 func (s *CollectionTestSuite) TestFetchCollection_FallbackToFirstLayer() {
 	collection := Collection{
 		Features: []CollectionFeature{
-			{ID: "fallback", Version: "1.0.0", Name: "Fallback"},
+			{ID: "fallback", Version: testVersion1_0_0, Name: "Fallback"},
 		},
 	}
 	data, err := json.Marshal(collection)

pkg/devcontainer/feature/extend.go

@@ -51,14 +51,22 @@ type BuildInfo struct {
 	BuildArgs               map[string]string
 }
 
-func GetExtendedBuildInfo(
-	ctx *config.SubstitutionContext,
-	imageBuildInfo *config.ImageBuildInfo,
-	target string,
-	devContainerConfig *config.SubstitutedConfig,
-	forceBuild bool,
-	secretOpts *SecretOptions,
-) (*ExtendedBuildInfo, error) {
+type ExtendedBuildInfoOptions struct {
+	Ctx                *config.SubstitutionContext
+	ImageBuildInfo     *config.ImageBuildInfo
+	Target             string
+	DevContainerConfig *config.SubstitutedConfig
+	ForceBuild         bool
+	SecretOpts         *SecretOptions
+}
+
+func GetExtendedBuildInfo(opts *ExtendedBuildInfoOptions) (*ExtendedBuildInfo, error) {
+	ctx := opts.Ctx
+	imageBuildInfo := opts.ImageBuildInfo
+	target := opts.Target
+	devContainerConfig := opts.DevContainerConfig
+	forceBuild := opts.ForceBuild
+	secretOpts := opts.SecretOpts
 	features, err := fetchFeatures(devContainerConfig.Config, forceBuild, secretOpts)
 	if err != nil {
 		return nil, fmt.Errorf("fetch features: %w", err)
@@ -219,22 +227,23 @@ func getFeatureSafeID(featureID string) string {
 }
 
 func getFeatureLayers(containerUser, remoteUser string, features []*config.FeatureSet) string {
-	result := `RUN \
-echo "_CONTAINER_USER_HOME=$(getent passwd ` + containerUser + ` | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env && \
-echo "_REMOTE_USER_HOME=$(getent passwd ` + remoteUser + ` | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env
+	var b strings.Builder
+	b.WriteString("RUN \\\n")
+	b.WriteString(`echo "_CONTAINER_USER_HOME=$(getent passwd ` + containerUser)
+	b.WriteString(` | cut -d: -f6)" >> ` +
+		`/tmp/build-features/devcontainer-features.builtin.env && \` + "\n")
+	b.WriteString(`echo "_REMOTE_USER_HOME=$(getent passwd ` + remoteUser)
+	b.WriteString(` | cut -d: -f6)" >> ` +
+		`/tmp/build-features/devcontainer-features.builtin.env` + "\n\n")
 
-`
 	for i, feature := range features {
-		result += generateContainerEnvs(feature)
-		result += `
-RUN cd /tmp/build-features/` + strconv.Itoa(i) + ` \
-&& chmod +x ./devcontainer-features-install.sh \
-&& ./devcontainer-features-install.sh
-
-`
+		b.WriteString(generateContainerEnvs(feature))
+		b.WriteString("\nRUN cd /tmp/build-features/" + strconv.Itoa(i) + ` \` + "\n")
+		b.WriteString("&& chmod +x ./devcontainer-features-install.sh \\\n")
+		b.WriteString("&& ./devcontainer-features-install.sh\n\n")
 	}
 
-	return result
+	return b.String()
 }
 
 func generateContainerEnvs(feature *config.FeatureSet) string {
@@ -254,32 +263,34 @@ func findContainerUsers(
 	composeServiceUser, imageUser string,
 ) (string, string) {
 	reversed := config.ReverseSlice(baseImageMetadata.Config)
-	containerUser := ""
-	remoteUser := ""
+	containerUser := resolveUserFromSources(
+		reversed, composeServiceUser, imageUser,
+		func(m *config.ImageMetadata) string { return m.ContainerUser },
+	)
+	remoteUser := resolveUserFromSources(
+		reversed, composeServiceUser, imageUser,
+		func(m *config.ImageMetadata) string { return m.RemoteUser },
+	)
+	return containerUser, remoteUser
+}
+
+func resolveUserFromSources(
+	reversed []*config.ImageMetadata,
+	composeServiceUser, imageUser string,
+	metadataField func(*config.ImageMetadata) string,
+) string {
 	for _, imageMetadata := range reversed {
-		if containerUser == "" && imageMetadata.ContainerUser != "" {
-			containerUser = imageMetadata.ContainerUser
-		}
-		if remoteUser == "" && imageMetadata.RemoteUser != "" {
-			remoteUser = imageMetadata.RemoteUser
+		if v := metadataField(imageMetadata); v != "" {
+			return v
 		}
 	}
-
-	if containerUser == "" {
-		if composeServiceUser != "" {
-			containerUser = composeServiceUser
-		} else if imageUser != "" {
-			containerUser = imageUser
-		}
+	if composeServiceUser != "" {
+		return composeServiceUser
 	}
-	if remoteUser == "" {
-		if composeServiceUser != "" {
-			remoteUser = composeServiceUser
-		} else if imageUser != "" {
-			remoteUser = imageUser
-		}
+	if imageUser != "" {
+		return imageUser
 	}
-	return containerUser, remoteUser
+	return ""
 }
 
 // ResolveFeatureOrder parses the features in a DevContainerConfig, resolves their
@@ -447,7 +458,7 @@ func (r *featureDependencyResolver) resolveFeatureDependency(
 	featureSet *config.FeatureSet,
 ) error {
 	if r.resolved[featureID] != nil {
-		return nil // Already resolved
+		return nil
 	}
 
 	if r.visiting[featureID] {
@@ -458,28 +469,12 @@ func (r *featureDependencyResolver) resolveFeatureDependency(
 	defer func() { r.visiting[featureID] = false }()
 
 	for depID, depOptions := range featureSet.Config.DependsOn {
-		normalizedDepID := normalizeFeatureID(depID)
-		resolvedKey, depFeatureSet := r.findByConfigID(normalizedDepID)
-		if depFeatureSet == nil {
-			if currentID, legacyMatch := r.legacyMap[normalizedDepID]; legacyMatch {
-				log.Debugf("resolved legacy ID %s to current feature %s", depID, currentID)
-				resolvedKey, depFeatureSet = r.findByConfigID(currentID)
-			}
-		}
-		if depFeatureSet == nil {
-			log.Debugf("installing dependency feature %s", depID)
-			var err error
-			depFeatureSet, err = r.processor.processFeature(depID, depOptions)
-			if err != nil {
-				return fmt.Errorf("failed to resolve dependency %s: %w", depID, err)
-			}
-			resolvedKey = featureDeduplicationKey(depFeatureSet.ConfigID, depFeatureSet.Version)
-			r.features[resolvedKey] = depFeatureSet
-			r.rebuildLegacyMap()
+		resolvedKey, depFeatureSet, err := r.findOrInstallDependency(depID, depOptions)
+		if err != nil {
+			return err
 		}
 
-		err := r.resolveFeatureDependency(resolvedKey, depFeatureSet)
-		if err != nil {
+		if err := r.resolveFeatureDependency(resolvedKey, depFeatureSet); err != nil {
 			return err
 		}
 	}
@@ -488,6 +483,31 @@ func (r *featureDependencyResolver) resolveFeatureDependency(
 	return nil
 }
 
+func (r *featureDependencyResolver) findOrInstallDependency(
+	depID string, depOptions any,
+) (string, *config.FeatureSet, error) {
+	normalizedDepID := normalizeFeatureID(depID)
+	resolvedKey, depFeatureSet := r.findByConfigID(normalizedDepID)
+	if depFeatureSet == nil {
+		if currentID, legacyMatch := r.legacyMap[normalizedDepID]; legacyMatch {
+			log.Debugf("resolved legacy ID %s to current feature %s", depID, currentID)
+			resolvedKey, depFeatureSet = r.findByConfigID(currentID)
+		}
+	}
+	if depFeatureSet == nil {
+		log.Debugf("installing dependency feature %s", depID)
+		var err error
+		depFeatureSet, err = r.processor.processFeature(depID, depOptions)
+		if err != nil {
+			return "", nil, fmt.Errorf("failed to resolve dependency %s: %w", depID, err)
+		}
+		resolvedKey = featureDeduplicationKey(depFeatureSet.ConfigID, depFeatureSet.Version)
+		r.features[resolvedKey] = depFeatureSet
+		r.rebuildLegacyMap()
+	}
+	return resolvedKey, depFeatureSet, nil
+}
+
 func (r *featureDependencyResolver) rebuildLegacyMap() {
 	r.legacyMap = buildLegacyIDMap(r.features)
 }