From 091044c7419b5cc679adbecc27d6901b0d1d10aa Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Fri, 26 Dec 2025 15:40:11 +0100 Subject: [PATCH] add script timeout, integration tests, and cleanup dead code - Add configurable timeout for script execution (default 30s) to prevent hangs from misbehaving scripts. Uses exec.CommandContext internally. - Remove unused excludeTagged() function and its test - dead code from previous refactor, functionality already exists in needsTag(). - Remove duplicate comment on getService() function. - Add integration test suite with 8 tests covering: - Basic tag update cycle - Tag cleanup - Run loop with dynamic tags - Real script execution - Service not found error handling - Empty script output (removes all prefixed tags) - Idempotency (no unnecessary Consul writes) - Service metadata preservation - Add docker-compose.test.yml with Consul 1.20 for local testing - Add Makefile with targets: test, test-integration, consul-up/down - Update CI workflow to run integration tests with Consul service container --- .github/workflows/ci.yml | 37 ++- .github/workflows/go.yml | 2 +- Makefile | 56 ++++ docker-compose.test.yml | 11 + go.mod | 25 +- go.sum | 66 +++-- pkg/tagit/integration_test.go | 471 ++++++++++++++++++++++++++++++++++ pkg/tagit/tagit.go | 36 +-- pkg/tagit/tagit_test.go | 73 ++---- 9 files changed, 654 insertions(+), 123 deletions(-) create mode 100644 Makefile create mode 100644 docker-compose.test.yml create mode 100644 pkg/tagit/integration_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5146c5c..27f05d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,7 +3,7 @@ name: Test and coverage on: [push, pull_request] jobs: - build: + unit-tests: runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 @@ -12,11 +12,40 @@ jobs: - uses: actions/setup-go@v6 with: go-version: '1.25' - - name: Run coverage - run: go test -coverpkg=./... ./... -race -coverprofile=coverage.out -covermode=atomic + - name: Run unit tests with coverage + run: go test -coverpkg=./... ./... -race -coverprofile=coverage.out -covermode=atomic - name: Upload coverage to Codecov uses: codecov/codecov-action@v5.5.1 with: - verbose: true + verbose: true env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + + integration-tests: + runs-on: ubuntu-latest + services: + consul: + image: hashicorp/consul:1.20 + ports: + - 8500:8500 + options: >- + --health-cmd "consul members" + --health-interval 5s + --health-timeout 5s + --health-retries 10 + steps: + - uses: actions/checkout@v5 + - uses: actions/setup-go@v6 + with: + go-version: '1.25' + - name: Wait for Consul + run: | + until curl -s http://localhost:8500/v1/status/leader | grep -q .; do + echo "Waiting for Consul..." + sleep 2 + done + echo "Consul is ready" + - name: Run integration tests + run: go test -race -tags=integration ./... + env: + CONSUL_ADDR: 127.0.0.1:8500 diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index d36ceed..545ca35 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -19,7 +19,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v4 with: - go-version: '1.22' + go-version: '1.25' - name: Build run: go build -v ./... diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..7a81a75 --- /dev/null +++ b/Makefile @@ -0,0 +1,56 @@ +.PHONY: test test-unit test-integration test-integration-ci consul-up consul-down build clean + +# Default target +all: test build + +# Build the binary +build: + go build -o tagit . + +# Run all tests (unit only by default) +test: test-unit + +# Run unit tests +test-unit: + go test -race ./... + +# Run integration tests (requires Consul running) +test-integration: + go test -race -tags=integration ./... + +# Start Consul for local integration testing +consul-up: + docker compose -f docker-compose.test.yml up -d + @echo "Waiting for Consul to be ready..." + @until docker compose -f docker-compose.test.yml exec -T consul consul members >/dev/null 2>&1; do \ + sleep 1; \ + done + @echo "Consul is ready" + +# Stop Consul +consul-down: + docker compose -f docker-compose.test.yml down -v + +# Run integration tests with Consul lifecycle management (for CI) +test-integration-ci: consul-up + @echo "Running integration tests..." + go test -race -tags=integration ./... || (docker compose -f docker-compose.test.yml down -v && exit 1) + docker compose -f docker-compose.test.yml down -v + +# Run all tests including integration (local development) +test-all: consul-up + go test -race -tags=integration ./... + docker compose -f docker-compose.test.yml down -v + +# Coverage with integration tests +coverage: + go test -coverpkg=./... ./... -race -coverprofile=coverage.out -covermode=atomic + +coverage-integration: consul-up + go test -coverpkg=./... -tags=integration ./... -race -coverprofile=coverage.out -covermode=atomic + docker compose -f docker-compose.test.yml down -v + +# Clean up +clean: + rm -f tagit coverage.out + docker compose -f docker-compose.test.yml down -v 2>/dev/null || true diff --git a/docker-compose.test.yml b/docker-compose.test.yml new file mode 100644 index 0000000..a507a9a --- /dev/null +++ b/docker-compose.test.yml @@ -0,0 +1,11 @@ +services: + consul: + image: hashicorp/consul:1.20 + command: agent -dev -client=0.0.0.0 + ports: + - "8500:8500" + healthcheck: + test: ["CMD", "consul", "members"] + interval: 2s + timeout: 5s + retries: 10 diff --git a/go.mod b/go.mod index e0b3fbb..1704878 100644 --- a/go.mod +++ b/go.mod @@ -1,14 +1,14 @@ module github.com/ncode/tagit -go 1.25 +go 1.25.3 require ( github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 - github.com/hashicorp/consul/api v1.32.1 - github.com/spf13/cobra v1.10.1 + github.com/hashicorp/consul/api v1.33.0 + github.com/spf13/cobra v1.10.2 github.com/spf13/pflag v1.0.10 - github.com/spf13/viper v1.20.1 - github.com/stretchr/testify v1.10.0 + github.com/spf13/viper v1.21.0 + github.com/stretchr/testify v1.11.1 ) require ( @@ -30,16 +30,15 @@ require ( github.com/mattn/go-colorable v0.1.14 // indirect github.com/mattn/go-isatty v0.0.20 // indirect github.com/mitchellh/go-homedir v1.1.0 // indirect - github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/pelletier/go-toml/v2 v2.2.4 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - github.com/sagikazarmark/locafero v0.10.0 // indirect - github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 // indirect - github.com/spf13/afero v1.14.0 // indirect - github.com/spf13/cast v1.9.2 // indirect + github.com/sagikazarmark/locafero v0.12.0 // indirect + github.com/spf13/afero v1.15.0 // indirect + github.com/spf13/cast v1.10.0 // indirect github.com/subosito/gotenv v1.6.0 // indirect - golang.org/x/exp v0.0.0-20250819193227-8b4c13bb791b // indirect - golang.org/x/sys v0.35.0 // indirect - golang.org/x/text v0.28.0 // indirect + go.yaml.in/yaml/v3 v3.0.4 // indirect + golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93 // indirect + golang.org/x/sys v0.39.0 // indirect + golang.org/x/text v0.32.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index e921e2b..5a60988 100644 --- a/go.sum +++ b/go.sum @@ -57,10 +57,10 @@ github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= -github.com/hashicorp/consul/api v1.32.1 h1:0+osr/3t/aZNAdJX558crU3PEjVrG4x6715aZHRgceE= -github.com/hashicorp/consul/api v1.32.1/go.mod h1:mXUWLnxftwTmDv4W3lzxYCPD199iNLLUyLfLGFJbtl4= -github.com/hashicorp/consul/sdk v0.16.1 h1:V8TxTnImoPD5cj0U9Spl0TUxcytjcbbJeADFF07KdHg= -github.com/hashicorp/consul/sdk v0.16.1/go.mod h1:fSXvwxB2hmh1FMZCNl6PwX0Q/1wdWtHJcZ7Ea5tns0s= +github.com/hashicorp/consul/api v1.33.0 h1:MnFUzN1Bo6YDGi/EsRLbVNgA4pyCymmcswrE5j4OHBM= +github.com/hashicorp/consul/api v1.33.0/go.mod h1:vLz2I/bqqCYiG0qRHGerComvbwSWKswc8rRFtnYBrIw= +github.com/hashicorp/consul/sdk v0.17.0 h1:N/JigV6y1yEMfTIhXoW0DXUecM2grQnFuRpY7PcLHLI= +github.com/hashicorp/consul/sdk v0.17.0/go.mod h1:8dgIhY6VlPUprRH7o7UenVuFEgq017qUn3k9wS5mCt4= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= @@ -128,8 +128,6 @@ github.com/miekg/dns v1.1.56 h1:5imZaSeoRNvpM9SzWNhEcP9QliKiz20/dA2QabIGVnE= github.com/miekg/dns v1.1.56/go.mod h1:cRm6Oo2C8TY9ZS/TqsSrseAcncm74lfK5G+ikN2SWWY= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= -github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY= -github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0= @@ -167,26 +165,24 @@ github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1 github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/sagikazarmark/locafero v0.10.0 h1:FM8Cv6j2KqIhM2ZK7HZjm4mpj9NBktLgowT1aN9q5Cc= -github.com/sagikazarmark/locafero v0.10.0/go.mod h1:Ieo3EUsjifvQu4NZwV5sPd4dwvu0OCgEQV7vjc9yDjw= +github.com/sagikazarmark/locafero v0.12.0 h1:/NQhBAkUb4+fH1jivKHWusDYFjMOOKU88eegjfxfHb4= +github.com/sagikazarmark/locafero v0.12.0/go.mod h1:sZh36u/YSZ918v0Io+U9ogLYQJ9tLLBmM4eneO6WwsI= github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529 h1:nn5Wsu0esKSJiIVhscUtVbo7ada43DJhG55ua/hjS5I= github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88= -github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 h1:+jumHNA0Wrelhe64i8F6HNlS8pkoyMv5sreGx2Ry5Rw= -github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8/go.mod h1:3n1Cwaq1E1/1lhQhtRK2ts/ZwZEhjcQeJQ1RuC6Q/8U= -github.com/spf13/afero v1.14.0 h1:9tH6MapGnn/j0eb0yIXiLjERO8RB6xIVZRDCX7PtqWA= -github.com/spf13/afero v1.14.0/go.mod h1:acJQ8t0ohCGuMN3O+Pv0V0hgMxNYDlvdk+VTfyZmbYo= -github.com/spf13/cast v1.9.2 h1:SsGfm7M8QOFtEzumm7UZrZdLLquNdzFYfIbEXntcFbE= -github.com/spf13/cast v1.9.2/go.mod h1:jNfB8QC9IA6ZuY2ZjDp0KtFO2LZZlg4S/7bzP6qqeHo= -github.com/spf13/cobra v1.10.1 h1:lJeBwCfmrnXthfAupyUTzJ/J4Nc1RsHC/mSRU2dll/s= -github.com/spf13/cobra v1.10.1/go.mod h1:7SmJGaTHFVBY0jW4NXGluQoLvhqFQM+6XSKD+P4XaB0= +github.com/spf13/afero v1.15.0 h1:b/YBCLWAJdFWJTN9cLhiXXcD7mzKn9Dm86dNnfyQw1I= +github.com/spf13/afero v1.15.0/go.mod h1:NC2ByUVxtQs4b3sIUphxK0NioZnmxgyCrfzeuq8lxMg= +github.com/spf13/cast v1.10.0 h1:h2x0u2shc1QuLHfxi+cTJvs30+ZAHOGRic8uyGTDWxY= +github.com/spf13/cast v1.10.0/go.mod h1:jNfB8QC9IA6ZuY2ZjDp0KtFO2LZZlg4S/7bzP6qqeHo= +github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= +github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= -github.com/spf13/viper v1.20.1 h1:ZMi+z/lvLyPSCoNtFCpqjy0S4kPbirhpTMwl8BkW9X4= -github.com/spf13/viper v1.20.1/go.mod h1:P9Mdzt1zoHIG8m2eZQinpiBjo6kCmZSKBClNNqjJvu4= +github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU= +github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= @@ -195,33 +191,35 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= -github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= -github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM= +go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= +go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/exp v0.0.0-20250819193227-8b4c13bb791b h1:DXr+pvt3nC887026GRP39Ej11UATqWDmWuS99x26cD0= -golang.org/x/exp v0.0.0-20250819193227-8b4c13bb791b/go.mod h1:4QTo5u+SEIbbKW1RacMZq1YEfOBqeXa19JeshGi+zc4= -golang.org/x/mod v0.27.0 h1:kb+q2PyFnEADO2IEF935ehFUXlWiNjJWtRNgBLSfbxQ= -golang.org/x/mod v0.27.0/go.mod h1:rWI627Fq0DEoudcK+MBkNkCe0EetEaDSwJJkCcjpazc= +golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93 h1:fQsdNF2N+/YewlRZiricy4P1iimyPKZ/xwniHj8Q2a0= +golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93/go.mod h1:EPRbTFwzwjXj9NpYyyrvenVh9Y+GFeEvMNh7Xuz7xgU= +golang.org/x/mod v0.31.0 h1:HaW9xtz0+kOcWKwli0ZXy79Ix+UW/vOfmWI5QVd2tgI= +golang.org/x/mod v0.31.0/go.mod h1:43JraMp9cGx1Rx3AqioxrbrhNsLl2l/iNAvuBkrezpg= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190613194153-d28f0bde5980/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200625001655-4c5254603344/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= -golang.org/x/net v0.38.0 h1:vRMAPTMaeGqVhG5QyLJHqNDwecKTomGeqbnfZyKlBI8= -golang.org/x/net v0.38.0/go.mod h1:ivrbrMbzFq5J41QOQh0siUuly180yBYtLp+CKbEaFx8= +golang.org/x/net v0.43.0 h1:lat02VYK2j4aLzMzecihNvTlJNQUq316m2Mr9rnM6YE= +golang.org/x/net v0.43.0/go.mod h1:vhO1fvI4dGsIjh73sWfUVjj3N7CA9WkKJNQm2svM6Jg= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw= -golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= +golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -240,15 +238,15 @@ golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220503163025-988cb79eb6c6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.35.0 h1:vz1N37gP5bs89s7He8XuIYXpyY0+QlsKmzipCbUtyxI= -golang.org/x/sys v0.35.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= +golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= +golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= -golang.org/x/text v0.28.0 h1:rhazDwis8INMIwQ4tpjLDzUhx6RlXqZNPEM0huQojng= -golang.org/x/text v0.28.0/go.mod h1:U8nCwOR8jO/marOQ0QbDiOngZVEBB7MAiitBuMjXiNU= +golang.org/x/text v0.32.0 h1:ZD01bjUt1FQ9WJ0ClOL5vxgxOI/sVCNgX1YtKwcY0mU= +golang.org/x/text v0.32.0/go.mod h1:o/rUWzghvpD5TXrTIBuJU77MTaN0ljMWE47kxGJQ7jY= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/tools v0.36.0 h1:kWS0uv/zsvHEle1LbV5LE8QujrxB3wfQyxHfhOk0Qkg= -golang.org/x/tools v0.36.0/go.mod h1:WBDiHKJK8YgLHlcQPYQzNCkUxUypCaa5ZegCVutKm+s= +golang.org/x/tools v0.40.0 h1:yLkxfA+Qnul4cs9QA3KnlFu0lVmd8JJfoq+E41uSutA= +golang.org/x/tools v0.40.0/go.mod h1:Ik/tzLRlbscWpqqMRjyWYDisX8bG13FrdXp3o4Sr9lc= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= diff --git a/pkg/tagit/integration_test.go b/pkg/tagit/integration_test.go new file mode 100644 index 0000000..0dd1970 --- /dev/null +++ b/pkg/tagit/integration_test.go @@ -0,0 +1,471 @@ +//go:build integration + +package tagit + +import ( + "context" + "io" + "log/slog" + "os" + "slices" + "testing" + "time" + + "github.com/hashicorp/consul/api" + "github.com/ncode/tagit/pkg/consul" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func getConsulAddr() string { + addr := os.Getenv("CONSUL_ADDR") + if addr == "" { + return "127.0.0.1:8500" + } + return addr +} + +func setupTestService(t *testing.T, client *api.Client, serviceID string, initialTags []string) { + t.Helper() + reg := &api.AgentServiceRegistration{ + ID: serviceID, + Name: serviceID, + Port: 8080, + Tags: initialTags, + } + err := client.Agent().ServiceRegister(reg) + require.NoError(t, err, "failed to register test service") + + t.Cleanup(func() { + _ = client.Agent().ServiceDeregister(serviceID) + }) +} + +func getServiceTags(t *testing.T, client *api.Client, serviceID string) []string { + t.Helper() + svc, _, err := client.Agent().Service(serviceID, nil) + require.NoError(t, err, "failed to get service") + require.NotNil(t, svc, "service not found") + return svc.Tags +} + +func TestIntegration_TagItRun(t *testing.T) { + consulClient, err := api.NewClient(&api.Config{ + Address: getConsulAddr(), + }) + require.NoError(t, err, "failed to create consul client") + + // Verify Consul is reachable + _, err = consulClient.Agent().Self() + require.NoError(t, err, "Consul not reachable at %s", getConsulAddr()) + + serviceID := "integration-test-service" + initialTags := []string{"existing-tag", "another-tag"} + + setupTestService(t, consulClient, serviceID, initialTags) + + // Create a mock executor that returns predictable output + mockExecutor := &MockCommandExecutor{ + MockOutput: []byte("tag1 tag2 tag3"), + MockError: nil, + } + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + wrappedClient, err := consul.CreateClient(getConsulAddr(), "") + require.NoError(t, err) + + tagit := New( + wrappedClient, + mockExecutor, + serviceID, + "echo tag1 tag2 tag3", // not actually used since we mock executor + 1*time.Second, + "test", + logger, + ) + + // Run one update cycle + err = tagit.updateServiceTags() + require.NoError(t, err) + + // Verify tags were applied + tags := getServiceTags(t, consulClient, serviceID) + slices.Sort(tags) + + expected := []string{"another-tag", "existing-tag", "test-tag1", "test-tag2", "test-tag3"} + slices.Sort(expected) + + assert.Equal(t, expected, tags, "tags should include original tags plus new prefixed tags") +} + +func TestIntegration_TagItCleanup(t *testing.T) { + consulClient, err := api.NewClient(&api.Config{ + Address: getConsulAddr(), + }) + require.NoError(t, err, "failed to create consul client") + + // Verify Consul is reachable + _, err = consulClient.Agent().Self() + require.NoError(t, err, "Consul not reachable at %s", getConsulAddr()) + + serviceID := "integration-cleanup-service" + initialTags := []string{"existing-tag", "test-tag1", "test-tag2", "other-tag"} + + setupTestService(t, consulClient, serviceID, initialTags) + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + wrappedClient, err := consul.CreateClient(getConsulAddr(), "") + require.NoError(t, err) + + tagit := New( + wrappedClient, + &CmdExecutor{}, + serviceID, + "", + 0, + "test", + logger, + ) + + // Run cleanup + err = tagit.CleanupTags() + require.NoError(t, err) + + // Verify prefixed tags were removed + tags := getServiceTags(t, consulClient, serviceID) + slices.Sort(tags) + + expected := []string{"existing-tag", "other-tag"} + slices.Sort(expected) + + assert.Equal(t, expected, tags, "only non-prefixed tags should remain") +} + +func TestIntegration_TagItRunLoop(t *testing.T) { + consulClient, err := api.NewClient(&api.Config{ + Address: getConsulAddr(), + }) + require.NoError(t, err, "failed to create consul client") + + // Verify Consul is reachable + _, err = consulClient.Agent().Self() + require.NoError(t, err, "Consul not reachable at %s", getConsulAddr()) + + serviceID := "integration-loop-service" + initialTags := []string{"existing-tag"} + + setupTestService(t, consulClient, serviceID, initialTags) + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + wrappedClient, err := consul.CreateClient(getConsulAddr(), "") + require.NoError(t, err) + + // Create executor that changes output over time + callCount := 0 + dynamicExecutor := &DynamicMockExecutor{ + ExecuteFunc: func(command string) ([]byte, error) { + callCount++ + if callCount == 1 { + return []byte("v1"), nil + } + return []byte("v2"), nil + }, + } + + tagit := New( + wrappedClient, + dynamicExecutor, + serviceID, + "echo dynamic", + 500*time.Millisecond, + "dyn", + logger, + ) + + ctx, cancel := context.WithTimeout(context.Background(), 1500*time.Millisecond) + defer cancel() + + // Run in background + done := make(chan struct{}) + go func() { + tagit.Run(ctx) + close(done) + }() + + // Wait for completion + <-done + + // Verify final state has updated tags + tags := getServiceTags(t, consulClient, serviceID) + + assert.Contains(t, tags, "existing-tag", "original tag should be preserved") + assert.Contains(t, tags, "dyn-v2", "final dynamic tag should be present") +} + +func TestIntegration_RealScriptExecution(t *testing.T) { + consulClient, err := api.NewClient(&api.Config{ + Address: getConsulAddr(), + }) + require.NoError(t, err, "failed to create consul client") + + // Verify Consul is reachable + _, err = consulClient.Agent().Self() + require.NoError(t, err, "Consul not reachable at %s", getConsulAddr()) + + serviceID := "integration-script-service" + initialTags := []string{"existing"} + + setupTestService(t, consulClient, serviceID, initialTags) + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + wrappedClient, err := consul.CreateClient(getConsulAddr(), "") + require.NoError(t, err) + + // Use real executor with actual shell command + tagit := New( + wrappedClient, + &CmdExecutor{Timeout: 5 * time.Second}, + serviceID, + "echo alpha beta gamma", + 1*time.Second, + "real", + logger, + ) + + // Run one update cycle + err = tagit.updateServiceTags() + require.NoError(t, err) + + // Verify tags were applied + tags := getServiceTags(t, consulClient, serviceID) + slices.Sort(tags) + + expected := []string{"existing", "real-alpha", "real-beta", "real-gamma"} + slices.Sort(expected) + + assert.Equal(t, expected, tags, "tags should include original plus script output tags") +} + +// DynamicMockExecutor allows changing behavior between calls +type DynamicMockExecutor struct { + ExecuteFunc func(command string) ([]byte, error) +} + +func (d *DynamicMockExecutor) Execute(command string) ([]byte, error) { + return d.ExecuteFunc(command) +} + +func TestIntegration_ServiceNotFound(t *testing.T) { + wrappedClient, err := consul.CreateClient(getConsulAddr(), "") + require.NoError(t, err) + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + tagit := New( + wrappedClient, + &MockCommandExecutor{MockOutput: []byte("tag1")}, + "non-existent-service-12345", + "echo tag1", + 1*time.Second, + "test", + logger, + ) + + // Should return error for non-existent service + err = tagit.updateServiceTags() + require.Error(t, err) + assert.Contains(t, err.Error(), "non-existent-service-12345") +} + +func TestIntegration_EmptyScriptOutput(t *testing.T) { + consulClient, err := api.NewClient(&api.Config{ + Address: getConsulAddr(), + }) + require.NoError(t, err) + + _, err = consulClient.Agent().Self() + require.NoError(t, err, "Consul not reachable") + + serviceID := "integration-empty-output-service" + // Start with some prefixed tags that should be removed + initialTags := []string{"keep-me", "test-remove1", "test-remove2", "also-keep"} + + setupTestService(t, consulClient, serviceID, initialTags) + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + wrappedClient, err := consul.CreateClient(getConsulAddr(), "") + require.NoError(t, err) + + // Empty output should remove all prefixed tags + tagit := New( + wrappedClient, + &MockCommandExecutor{MockOutput: []byte("")}, + serviceID, + "echo", + 1*time.Second, + "test", + logger, + ) + + err = tagit.updateServiceTags() + require.NoError(t, err) + + tags := getServiceTags(t, consulClient, serviceID) + slices.Sort(tags) + + // Only non-prefixed tags should remain + expected := []string{"also-keep", "keep-me"} + assert.Equal(t, expected, tags, "prefixed tags should be removed when script outputs nothing") +} + +func TestIntegration_Idempotency(t *testing.T) { + consulClient, err := api.NewClient(&api.Config{ + Address: getConsulAddr(), + }) + require.NoError(t, err) + + _, err = consulClient.Agent().Self() + require.NoError(t, err, "Consul not reachable") + + serviceID := "integration-idempotent-service" + initialTags := []string{"existing"} + + setupTestService(t, consulClient, serviceID, initialTags) + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + wrappedClient, err := consul.CreateClient(getConsulAddr(), "") + require.NoError(t, err) + + mockExecutor := &MockCommandExecutor{ + MockOutput: []byte("stable-tag"), + } + + tagit := New( + wrappedClient, + mockExecutor, + serviceID, + "echo stable-tag", + 1*time.Second, + "idem", + logger, + ) + + // First update should register + err = tagit.updateServiceTags() + require.NoError(t, err) + + // Get the current modify index to detect changes + svc1, _, err := consulClient.Agent().Service(serviceID, nil) + require.NoError(t, err) + modifyIndex1 := svc1.ModifyIndex + + // Second update with same output should NOT register + err = tagit.updateServiceTags() + require.NoError(t, err) + + svc2, _, err := consulClient.Agent().Service(serviceID, nil) + require.NoError(t, err) + modifyIndex2 := svc2.ModifyIndex + + // Third update with same output should NOT register + err = tagit.updateServiceTags() + require.NoError(t, err) + + svc3, _, err := consulClient.Agent().Service(serviceID, nil) + require.NoError(t, err) + modifyIndex3 := svc3.ModifyIndex + + // ModifyIndex should not change after first update since tags are identical + assert.Equal(t, modifyIndex1, modifyIndex2, "second update should not modify service") + assert.Equal(t, modifyIndex2, modifyIndex3, "third update should not modify service") + + // Verify correct tags are present + tags := getServiceTags(t, consulClient, serviceID) + slices.Sort(tags) + expected := []string{"existing", "idem-stable-tag"} + assert.Equal(t, expected, tags) +} + +func TestIntegration_ServiceMetadataPreservation(t *testing.T) { + consulClient, err := api.NewClient(&api.Config{ + Address: getConsulAddr(), + }) + require.NoError(t, err) + + _, err = consulClient.Agent().Self() + require.NoError(t, err, "Consul not reachable") + + serviceID := "integration-metadata-service" + + // Register service with metadata, weights, and other fields + reg := &api.AgentServiceRegistration{ + ID: serviceID, + Name: "metadata-test", + Port: 9090, + Address: "10.0.0.1", + Tags: []string{"original-tag"}, + Meta: map[string]string{ + "version": "1.2.3", + "environment": "test", + "owner": "integration-test", + }, + Weights: &api.AgentWeights{ + Passing: 10, + Warning: 5, + }, + } + err = consulClient.Agent().ServiceRegister(reg) + require.NoError(t, err) + + t.Cleanup(func() { + _ = consulClient.Agent().ServiceDeregister(serviceID) + }) + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + wrappedClient, err := consul.CreateClient(getConsulAddr(), "") + require.NoError(t, err) + + tagit := New( + wrappedClient, + &MockCommandExecutor{MockOutput: []byte("new-tag")}, + serviceID, + "echo new-tag", + 1*time.Second, + "meta", + logger, + ) + + // Update tags + err = tagit.updateServiceTags() + require.NoError(t, err) + + // Verify all original properties are preserved + svc, _, err := consulClient.Agent().Service(serviceID, nil) + require.NoError(t, err) + + assert.Equal(t, "metadata-test", svc.Service, "service name should be preserved") + assert.Equal(t, 9090, svc.Port, "port should be preserved") + assert.Equal(t, "10.0.0.1", svc.Address, "address should be preserved") + + // Verify metadata is preserved + assert.Equal(t, "1.2.3", svc.Meta["version"], "metadata version should be preserved") + assert.Equal(t, "test", svc.Meta["environment"], "metadata environment should be preserved") + assert.Equal(t, "integration-test", svc.Meta["owner"], "metadata owner should be preserved") + + // Verify weights are preserved + assert.Equal(t, 10, svc.Weights.Passing, "passing weight should be preserved") + assert.Equal(t, 5, svc.Weights.Warning, "warning weight should be preserved") + + // Verify tags were updated correctly + tags := svc.Tags + slices.Sort(tags) + expected := []string{"meta-new-tag", "original-tag"} + assert.Equal(t, expected, tags, "tags should include original plus new prefixed tag") +} diff --git a/pkg/tagit/tagit.go b/pkg/tagit/tagit.go index cb68660..c59d7e2 100644 --- a/pkg/tagit/tagit.go +++ b/pkg/tagit/tagit.go @@ -30,7 +30,12 @@ type CommandExecutor interface { Execute(command string) ([]byte, error) } -type CmdExecutor struct{} +// DefaultScriptTimeout is the default timeout for script execution. +const DefaultScriptTimeout = 30 * time.Second + +type CmdExecutor struct { + Timeout time.Duration +} func (e *CmdExecutor) Execute(command string) ([]byte, error) { if command == "" { @@ -43,7 +48,20 @@ func (e *CmdExecutor) Execute(command string) ([]byte, error) { if len(args) == 0 { return nil, fmt.Errorf("failed to execute: no command after splitting") } - return exec.Command(args[0], args[1:]...).Output() + + timeout := e.Timeout + if timeout == 0 { + timeout = DefaultScriptTimeout + } + + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + out, err := exec.CommandContext(ctx, args[0], args[1:]...).Output() + if ctx.Err() == context.DeadlineExceeded { + return nil, fmt.Errorf("script execution timed out after %v", timeout) + } + return out, err } // New creates a new TagIt struct. @@ -180,7 +198,6 @@ func (t *TagIt) copyServiceToRegistration(service *api.AgentService) *api.AgentS return registration } -// getService returns the registered service. // getService returns the registered service. func (t *TagIt) getService() (*api.AgentService, error) { agent := t.client.Agent() @@ -221,19 +238,6 @@ func (t *TagIt) needsTag(current []string, update []string) (updatedTags []strin return updatedTags, true } -// excludeTagged filters out tags that are already tagged with the prefix. -func (t *TagIt) excludeTagged(tags []string) (filteredTags []string, tagged bool) { - filteredTags = make([]string, 0) // Initialize with empty slice instead of nil - for _, tag := range tags { - if strings.HasPrefix(tag, t.TagPrefix+"-") { - tagged = true - } else { - filteredTags = append(filteredTags, tag) - } - } - return filteredTags, tagged -} - // diffTags compares two slices of strings and returns the difference. func (t *TagIt) diffTags(current, update []string) []string { diff := make([]string, 0) diff --git a/pkg/tagit/tagit_test.go b/pkg/tagit/tagit_test.go index ea85a98..9c28681 100644 --- a/pkg/tagit/tagit_test.go +++ b/pkg/tagit/tagit_test.go @@ -97,61 +97,6 @@ func TestDiffTags(t *testing.T) { } } -func TestExcludeTagged(t *testing.T) { - tests := []struct { - name string - tags []string - tagPrefix string - expected []string - shouldTag bool - }{ - { - name: "No Tags With Prefix", - tags: []string{"alpha", "beta", "gamma"}, - tagPrefix: "tag", - expected: []string{"alpha", "beta", "gamma"}, - shouldTag: false, - }, - { - name: "All Tags With Prefix", - tags: []string{"tag-alpha", "tag-beta", "tag-gamma"}, - tagPrefix: "tag", - expected: []string{}, - shouldTag: true, - }, - { - name: "Some Tags With Prefix", - tags: []string{"alpha", "tag-beta", "gamma"}, - tagPrefix: "tag", - expected: []string{"alpha", "gamma"}, - shouldTag: true, - }, - { - name: "Empty Tags", - tags: []string{}, - tagPrefix: "tag", - expected: []string{}, - shouldTag: false, - }, - { - name: "Prefix in Middle", - tags: []string{"alpha-tag", "beta", "gamma"}, - tagPrefix: "tag", - expected: []string{"alpha-tag", "beta", "gamma"}, - shouldTag: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tagit := TagIt{TagPrefix: tt.tagPrefix} - filteredTags, tagged := tagit.excludeTagged(tt.tags) - assert.Equal(t, tt.expected, filteredTags, "excludeTagged() returned unexpected filtered tags") - assert.Equal(t, tt.shouldTag, tagged, "excludeTagged() returned unexpected shouldTag value") - }) - } -} - func TestNeedsTag(t *testing.T) { tests := []struct { name string @@ -684,3 +629,21 @@ func TestCmdExecutor_Execute(t *testing.T) { }) } } + +func TestCmdExecutor_Timeout(t *testing.T) { + executor := &CmdExecutor{Timeout: 100 * time.Millisecond} + + _, err := executor.Execute("sleep 5") + assert.Error(t, err) + assert.Contains(t, err.Error(), "timed out") +} + +func TestCmdExecutor_DefaultTimeout(t *testing.T) { + executor := &CmdExecutor{} + assert.Equal(t, time.Duration(0), executor.Timeout, "Timeout should be zero before execution") + + // Quick command should succeed with default timeout + output, err := executor.Execute("echo hello") + assert.NoError(t, err) + assert.Equal(t, "hello\n", string(output)) +}