Skip to content

Commit

Permalink
Merge pull request containers#5239 from serprex/strings-cut
Browse files Browse the repository at this point in the history 
Replace strings.SplitN with strings.Cut
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Jan 3, 2024
2 parents 51fc7f2 + 53c65dd commit e56cb8e
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 131 deletions.
49 changes: 18 additions & 31 deletions imagebuildah/executor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,13 +337,13 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o
// We have to be careful here - it's either an argument
// and value, or just an argument, since they can be
// separated by either "=" or whitespace.
list := strings.SplitN(arg.Value, "=", 2)
argName, argValue, hasValue := strings.Cut(arg.Value, "=")
if !foundFirstStage {
if len(list) > 1 {
globalArgs[list[0]] = list[1]
if hasValue {
globalArgs[argName] = argValue
}
}
delete(exec.unusedArgs, list[0])
delete(exec.unusedArgs, argName)
}
case "FROM":
foundFirstStage = true
Expand Down Expand Up @@ -496,12 +496,7 @@ func (b *Executor) buildStage(ctx context.Context, cleanupStages map[int]*StageE
var labelLine string
labels := append([]string{}, b.labels...)
for _, labelSpec := range labels {
label := strings.SplitN(labelSpec, "=", 2)
key := label[0]
value := ""
if len(label) > 1 {
value = label[1]
}
key, value, _ := strings.Cut(labelSpec, "=")
// check only for an empty key since docker allows empty values
if key != "" {
labelLine += fmt.Sprintf(" %q=%q", key, value)
Expand All @@ -523,10 +518,8 @@ func (b *Executor) buildStage(ctx context.Context, cleanupStages map[int]*StageE
if len(b.envs) > 0 {
var envLine string
for _, envSpec := range b.envs {
env := strings.SplitN(envSpec, "=", 2)
key := env[0]
if len(env) > 1 {
value := env[1]
key, value, hasValue := strings.Cut(envSpec, "=")
if hasValue {
envLine += fmt.Sprintf(" %q=%q", key, value)
} else {
return "", nil, false, fmt.Errorf("BUG: unresolved environment variable: %q", key)
Expand Down Expand Up @@ -844,24 +837,18 @@ func (b *Executor) Build(ctx context.Context, stages imagebuilder.Stages) (image
mountFlags := strings.TrimPrefix(flag, "--mount=")
fields := strings.Split(mountFlags, ",")
for _, field := range fields {
if strings.HasPrefix(field, "from=") {
fromField := strings.SplitN(field, "=", 2)
if len(fromField) > 1 {
mountFrom := fromField[1]
// Check if this base is a stage if yes
// add base to current stage's dependency tree
// but also confirm if this is not in additional context.
if _, ok := b.additionalBuildContexts[mountFrom]; !ok {
// Treat from as a rootfs we need to preserve
b.rootfsMap[mountFrom] = true
if _, ok := dependencyMap[mountFrom]; ok {
// update current stage's dependency info
currentStageInfo := dependencyMap[stage.Name]
currentStageInfo.Needs = append(currentStageInfo.Needs, mountFrom)
}
if mountFrom, hasFrom := strings.CutPrefix(field, "from="); hasFrom {
// Check if this base is a stage if yes
// add base to current stage's dependency tree
// but also confirm if this is not in additional context.
if _, ok := b.additionalBuildContexts[mountFrom]; !ok {
// Treat from as a rootfs we need to preserve
b.rootfsMap[mountFrom] = true
if _, ok := dependencyMap[mountFrom]; ok {
// update current stage's dependency info
currentStageInfo := dependencyMap[stage.Name]
currentStageInfo.Needs = append(currentStageInfo.Needs, mountFrom)
}
} else {
return "", nil, fmt.Errorf("invalid value for field `from=`: %q", fromField[1])
}
}
}
Expand Down
45 changes: 18 additions & 27 deletions imagebuildah/stage_executor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,24 +565,23 @@ func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]inte
stageMountPoints := make(map[string]internal.StageMountDetails)
for _, flag := range mountList {
if strings.Contains(flag, "from") {
arr := strings.SplitN(flag, ",", 2)
if len(arr) < 2 {
tokens := strings.Split(flag, ",")
if len(tokens) < 2 {
return nil, fmt.Errorf("Invalid --mount command: %s", flag)
}
tokens := strings.Split(flag, ",")
for _, val := range tokens {
kv := strings.SplitN(val, "=", 2)
switch kv[0] {
for _, token := range tokens {
key, val, hasVal := strings.Cut(token, "=")
switch key {
case "from":
if len(kv) == 1 {
if !hasVal {
return nil, fmt.Errorf("unable to resolve argument for `from=`: bad argument")
}
if kv[1] == "" {
if val == "" {
return nil, fmt.Errorf("unable to resolve argument for `from=`: from points to an empty value")
}
from, fromErr := imagebuilder.ProcessWord(kv[1], s.stage.Builder.Arguments())
from, fromErr := imagebuilder.ProcessWord(val, s.stage.Builder.Arguments())
if fromErr != nil {
return nil, fmt.Errorf("unable to resolve argument %q: %w", kv[1], fromErr)
return nil, fmt.Errorf("unable to resolve argument %q: %w", val, fromErr)
}
// If additional buildContext contains this
// give priority to that and break if additional
Expand Down Expand Up @@ -1748,9 +1747,9 @@ func (s *StageExecutor) getBuildArgsResolvedForRun() string {
dockerConfig := s.stage.Builder.Config()

for _, env := range dockerConfig.Env {
splitv := strings.SplitN(env, "=", 2)
if len(splitv) == 2 {
configuredEnvs[splitv[0]] = splitv[1]
key, val, hasVal := strings.Cut(env, "=")
if hasVal {
configuredEnvs[key] = val
}
}

Expand Down Expand Up @@ -2102,8 +2101,8 @@ func (s *StageExecutor) commit(ctx context.Context, createdBy string, emptyLayer
s.builder.SetPort(string(p))
}
for _, envSpec := range config.Env {
spec := strings.SplitN(envSpec, "=", 2)
s.builder.SetEnv(spec[0], spec[1])
key, val, _ := strings.Cut(envSpec, "=")
s.builder.SetEnv(key, val)
}
for _, envSpec := range s.executor.unsetEnvs {
s.builder.UnsetEnv(envSpec)
Expand Down Expand Up @@ -2139,12 +2138,8 @@ func (s *StageExecutor) commit(ctx context.Context, createdBy string, emptyLayer
// an intermediate image, in such case we must
// honor layer labels if they are configured.
for _, labelString := range s.executor.layerLabels {
label := strings.SplitN(labelString, "=", 2)
if len(label) > 1 {
s.builder.SetLabel(label[0], label[1])
} else {
s.builder.SetLabel(label[0], "")
}
labelk, labelv, _ := strings.Cut(labelString, "=")
s.builder.SetLabel(labelk, labelv)
}
}
for k, v := range config.Labels {
Expand All @@ -2157,12 +2152,8 @@ func (s *StageExecutor) commit(ctx context.Context, createdBy string, emptyLayer
s.builder.UnsetLabel(key)
}
for _, annotationSpec := range s.executor.annotations {
annotation := strings.SplitN(annotationSpec, "=", 2)
if len(annotation) > 1 {
s.builder.SetAnnotation(annotation[0], annotation[1])
} else {
s.builder.SetAnnotation(annotation[0], "")
}
annotationk, annotationv, _ := strings.Cut(annotationSpec, "=")
s.builder.SetAnnotation(annotationk, annotationv)
}
if imageRef != nil {
logName := transports.ImageName(imageRef)
Expand Down
10 changes: 5 additions & 5 deletions internal/source/add.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ func (o *AddOptions) annotations() (map[string]string, error) {
annotations := make(map[string]string)

for _, unparsed := range o.Annotations {
parsed := strings.SplitN(unparsed, "=", 2)
if len(parsed) != 2 {
key, value, hasValue := strings.Cut(unparsed, "=")
if !hasValue {
return nil, fmt.Errorf("invalid annotation %q (expected format is \"key=value\")", unparsed)
}
if _, exists := annotations[parsed[0]]; exists {
return nil, fmt.Errorf("annotation %q specified more than once", parsed[0])
if _, exists := annotations[key]; exists {
return nil, fmt.Errorf("annotation %q specified more than once", key)
}
annotations[parsed[0]] = parsed[1]
annotations[key] = value
}

return annotations, nil
Expand Down
Loading

0 comments on commit e56cb8e

Please sign in to comment.