Skip to content

Conversation

@psturc
Copy link
Collaborator

@psturc psturc commented Jul 4, 2025

Description

While testing a konflux running on kind cluster, we found out it'd be good to have additional ports mapped in aws so other services can be publicly accessible (e.g. tekton-results, tekton-pipelines, etc).

This change enables adding extra port mappings via new flag.

Example:
--extra-port-mappings='[{\"containerPort\": 8080, \"hostPort\": 8080, \"protocol\": \"TCP\"}]'

Verification

Ran the mapt create command with following param:

--extra-port-mappings='[{"containerPort": 30012, "hostPort": 8080, "protocol": "TCP"}, {"containerPort": 30013, "hostPort": 9000, "protocol": "TCP"}]'

And verified that the new ports appear in the EC2 inbound rules

Screenshot 2025-07-15 at 12 07 19

and that the endpoints are accessible via exposed ports

@psturc
Copy link
Collaborator Author

psturc commented Jul 10, 2025

@adrianriobo I plan to finish this soon. What do you think of this approach so far?

@adrianriobo
Copy link
Collaborator

Yeah I think the feature makes totally sense, and I checked the code quick and it seems totally fine.... just for the cmd part there is an specific type you can use to avoid all the parsing..

check this:

https://github.com/adrianriobo/mapt/blob/ef86305923e0d9bddee44771d3e52c90cd7a8107/cmd/mapt/cmd/params/params.go#L65

https://github.com/adrianriobo/mapt/blob/ef86305923e0d9bddee44771d3e52c90cd7a8107/cmd/mapt/cmd/params/params.go#L140

https://github.com/adrianriobo/mapt/blob/fix-406/cmd/mapt/cmd/params/params.go#L127

This will simplify the code in that part

@adrianriobo
Copy link
Collaborator

Rebase as I changed some file names and specs to facilitate spot management 🙏

@psturc psturc force-pushed the proposal-add-extra-ports branch from 3e4f3ec to 79d97e9 Compare July 14, 2025 15:39
@psturc psturc changed the title WIP: extra port mapping for kind cluster feat: extra port mapping for kind cluster Jul 15, 2025
@psturc psturc marked this pull request as ready for review July 15, 2025 10:05
@psturc psturc requested a review from adrianriobo July 15, 2025 10:49
@adrianriobo
Copy link
Collaborator

adrianriobo commented Jul 15, 2025

Just one thing...did you try to parse the struct as json ...so we can avoid the escape chars... actually is kind of the same you did in the action, mm if that is the case then you can use that struct in the Args of the action.

If you already try just say and I will LGTM the PR

something like:

package main

import (
	"encoding/json"
	"fmt"
	"log"

	"github.com/spf13/pflag"
	"github.com/spf13/viper"
)

type Config struct {
	Name   string   `json:"name"`
	Enabled bool     `json:"enabled"`
	Items  []string `json:"items"`
}

func main() {
	// Define a flag for the JSON config
	pflag.String("config", "", "JSON string with configuration")
	pflag.Parse()

	// Bind pflag to viper
	err := viper.BindPFlags(pflag.CommandLine)
	if err != nil {
		log.Fatalf("error binding flags: %v", err)
	}

	// Get the JSON string
	configStr := viper.GetString("config")
	if configStr == "" {
		log.Fatal("missing --config argument")
	}

	// Unmarshal JSON string into struct
	var cfg Config
	err = json.Unmarshal([]byte(configStr), &cfg)
	if err != nil {
		log.Fatalf("error parsing config JSON: %v", err)
	}

	// Use the config
	fmt.Printf("Parsed config: %+v\n", cfg)
}

Copy link
Collaborator

@adrianriobo adrianriobo left a comment

Choose a reason for hiding this comment

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

LGTM when it is merged I will give a try to the parse at cmd level no worries

@adrianriobo
Copy link
Collaborator

BTW did you have the chance to test this?

@psturc
Copy link
Collaborator Author

psturc commented Jul 15, 2025

yep, tested it like this:

mapt aws kind create --project-name kind-psturc-test-extra-params-1 \
--backed-url "s3://mapt-kind-bucket/mapt/kind/psturc-test-extra-params-1-$(date -u +"%s")" \
--conn-details-output /tmp/ --arch x86_64 --cpus 32 --memory 64 --version v1.32 --spot --spot-increase-rate 20 \
--timeout 20m --tags env=konflux,user=psturc \
--extra-port-mappings='[{"containerPort": 30012, "hostPort": 8080, "protocol": "TCP"}, {"containerPort": 30013, "hostPort": 9000, "protocol": "TCP"}]'

also added a validation of provided input values for the new flag

@adrianriobo adrianriobo merged commit d756f59 into redhat-developer:main Jul 16, 2025
7 checks passed
@psturc psturc deleted the proposal-add-extra-ports branch July 16, 2025 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants