Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
name: Build metering-test-adapter image
name: Build metering-echo-adapter image

on:
workflow_dispatch:
pull_request:
paths:
- 'osac-metering/metering-service/**'
- '!osac-metering/metering-service/**/*.md'
- 'osac-metering/adapters/**'
- '!osac-metering/adapters/**/*.md'
push:
branches:
- main
Expand All @@ -18,7 +18,7 @@ concurrency:

env:
REGISTRY: ghcr.io
IMAGE_NAME: osac-project/metering-test-adapter
IMAGE_NAME: osac-project/metering-echo-adapter

jobs:
build:
Expand Down Expand Up @@ -70,8 +70,8 @@ jobs:
- name: Build and push Docker image
uses: docker/build-push-action@v7
with:
context: osac-metering/metering-service
file: osac-metering/metering-service/Containerfile.test-adapter
context: osac-metering/adapters
file: osac-metering/adapters/Containerfile.echo-adapter
push: ${{ github.event_name != 'pull_request' }}
tags: ${{ steps.meta.outputs.tags }}
labels: ${{ steps.meta.outputs.labels }}
2 changes: 1 addition & 1 deletion .github/workflows/e2e-vmaas-full-install.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ jobs:
# override mechanism keys strictly off imageKey, one build per key, so this
# is the only way to get both fields covered without changing that mechanism
# (which lives in osac-test-infra, not here). See OSAC-3546.
components: '[{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"fulfillment-service/Containerfile","context":".","imageKey":"service.images.service"},{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-operator/Containerfile","context":".","imageKey":"operator.image.repository"},{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-aap/execution-environment/execution-environment.yaml","imageKey":"aap.bootstrap.image","buildType":"ansible-builder"},{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-aap/execution-environment/execution-environment.yaml","imageKey":"aap.configAsCode.eeImage","buildType":"ansible-builder"},{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"bare-metal-fulfillment-operator/Containerfile","context":".","imageKey":"bmf.image.repository"},{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-metering/metering-service/Containerfile","imageKey":"metering.image.repository"},{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-metering/metering-service/Containerfile.test-adapter","imageKey":"metering.testAdapter.image.repository"}]'
components: '[{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"fulfillment-service/Containerfile","context":".","imageKey":"service.images.service"},{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-operator/Containerfile","context":".","imageKey":"operator.image.repository"},{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-aap/execution-environment/execution-environment.yaml","imageKey":"aap.bootstrap.image","buildType":"ansible-builder"},{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-aap/execution-environment/execution-environment.yaml","imageKey":"aap.configAsCode.eeImage","buildType":"ansible-builder"},{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"bare-metal-fulfillment-operator/Containerfile","context":".","imageKey":"bmf.image.repository"},{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-metering/metering-service/Containerfile","imageKey":"metering.image.repository"},{"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-metering/adapters/Containerfile.echo-adapter","imageKey":"metering.echoAdapter.image.repository"}]'
# Pass author_association for fork PR authorization (empty for non-fork PRs)
fork-pr-author-association: ${{ github.event.pull_request.head.repo.fork && github.event.pull_request.author_association || '' }}
fork-pr-author: ${{ github.event.pull_request.head.repo.fork && github.event.pull_request.user.login || '' }}
Expand Down
16 changes: 16 additions & 0 deletions .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,19 @@ jobs:
- working-directory: osac-metering/metering-service
run: |
ginkgo run -r internal

run-osac-metering-adapters-tests:
name: Run unit tests (osac-metering/adapters)
needs: changes
if: needs.changes.outputs.should-run == 'true'
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
- uses: ./.github/actions/setup-go
with:
working-directory: osac-metering/adapters
- working-directory: osac-metering/adapters
run: |
ginkgo run .
Comment on lines +88 to +102

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect all commit trailers and AI-tool references.
git log --format='%H%n%B' |
  rg -n -i '^(Assisted-by|Generated-by|Co-Authored-By):|coderabbit|openai|copilot|claude|ai[- ]generated' || true

Repository: osac-project/osac

Length of output: 50374


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import subprocess, re

raw = subprocess.check_output(
    ["git", "log", "--format=%H%x00%B"], text=True, errors="replace"
)
commits = raw.split("\x00")
ai_trailer_rx = re.compile(r'(?mi)^([A-Za-z][A-Za-z0-9-]*-[A-Za-z][A-Za-z0-9-]*)\s*:\s*(.*?)(?:<[^>]+>)?\s*$', re.DEBUG)
co_authored_rx = re.compile(r'(?mi)^Co-Authored-By\s*:\s*(.*?)(?:<[^>]+>)?\s*$', re.DEBUG | re.DEBUG)

tools = ["CodeRabbit", "OpenAI", "GitHub Copilot", "Copilot", "Claude", "Cursor", "Claude Code", "AI"]
ai_trailer_seen = 0
co_authored_ai = 0
for commit in commits:
    if not commit.strip():
        continue
    b = commit.splitlines()
    seen = any(ai_trailer_rx.match(line) for line in b)
    ai_trailer_seen += seen
    co = co_authored_rx.findall('\n'.join(b))
    for value in co:
        if any(t in value for t in tools):
            co_authored_ai += 1
            print("AI in Co-Authored-By:", value)

print("commits_with_ai_trailer:", ai_trailer_seen)
print("co_authored_ai_tools:", co_authored_ai)
PY

printf '\nRecent commits: '
git rev-parse HEAD
git --no-pager log --oneline -3

Repository: osac-project/osac

Length of output: 25770


Remove AI tools from Co-Authored-By.

Use Assisted-by/Generated-by for AI assistance, and do not list AI tools such as Claude/Cursor in Co-Authored-By.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/unit-tests.yml around lines 88 - 102, Update the commit
metadata associated with the osac-metering/adapters workflow changes to remove
any AI tools from Co-Authored-By trailers, using Assisted-by or Generated-by
instead where AI assistance must be acknowledged.

Source: Coding guidelines

6 changes: 6 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,9 @@ repos:
language: system
files: ^osac-metering/metering-service/.*\.go$
entry: make -C osac-metering/metering-service lint
- id: osac-metering-adapters-golangci-lint
pass_filenames: false
name: osac-metering-adapters golangci-lint
language: system
files: ^osac-metering/adapters/(.*\.go|\.golangci\.yml|Makefile|go\.(mod|sum))$
entry: make -C osac-metering/adapters lint
Comment thread
coderabbitai[bot] marked this conversation as resolved.
1 change: 1 addition & 0 deletions bare-metal-fulfillment-operator/Containerfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ COPY --chown=1001:1001 osac-operator/api/go.mod osac-operator/api/go.sum osac-op
COPY --chown=1001:1001 fulfillment-service/go.mod fulfillment-service/go.sum fulfillment-service/
COPY --chown=1001:1001 osac-csi-driver/go.mod osac-csi-driver/go.sum osac-csi-driver/
COPY --chown=1001:1001 osac-metering/metering-service/go.mod osac-metering/metering-service/go.sum osac-metering/metering-service/
COPY --chown=1001:1001 osac-metering/adapters/go.mod osac-metering/adapters/go.sum osac-metering/adapters/
Comment thread
coderabbitai[bot] marked this conversation as resolved.
RUN cd bare-metal-fulfillment-operator && go mod download

COPY --chown=1001:1001 bare-metal-fulfillment-operator/ bare-metal-fulfillment-operator/
Expand Down
1 change: 1 addition & 0 deletions fulfillment-service/Containerfile
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ COPY --chown=1001:1001 osac-operator/go.mod osac-operator/go.sum osac-operator/
COPY --chown=1001:1001 osac-operator/api/go.mod osac-operator/api/go.sum osac-operator/api/
COPY --chown=1001:1001 osac-csi-driver/go.mod osac-csi-driver/go.sum osac-csi-driver/
COPY --chown=1001:1001 osac-metering/metering-service/go.mod osac-metering/metering-service/go.sum osac-metering/metering-service/
COPY --chown=1001:1001 osac-metering/adapters/go.mod osac-metering/adapters/go.sum osac-metering/adapters/
RUN \
set -e; \
cd fulfillment-service && go mod download
Expand Down
1 change: 1 addition & 0 deletions go.work
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use (
./bare-metal-fulfillment-operator
./fulfillment-service
./osac-csi-driver
./osac-metering/adapters
./osac-metering/metering-service
./osac-operator
./osac-operator/api
Expand Down
1 change: 1 addition & 0 deletions osac-csi-driver/Containerfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ COPY --chown=1001:1001 bare-metal-fulfillment-operator/go.mod bare-metal-fulfill
COPY --chown=1001:1001 osac-operator/go.mod osac-operator/go.sum osac-operator/
COPY --chown=1001:1001 osac-operator/api/go.mod osac-operator/api/go.sum osac-operator/api/
COPY --chown=1001:1001 osac-metering/metering-service/go.mod osac-metering/metering-service/go.sum osac-metering/metering-service/
COPY --chown=1001:1001 osac-metering/adapters/go.mod osac-metering/adapters/go.sum osac-metering/adapters/
RUN cd osac-csi-driver && go mod download

COPY --chown=1001:1001 osac-csi-driver/ osac-csi-driver/
Expand Down
8 changes: 7 additions & 1 deletion osac-installer/values/vmaas-ci/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,14 @@ kafka:
# --- Metering ---
metering:
enabled: true
testAdapter:
# Echo adapter — test/development tool only. Consumes metering events from
# Kafka and exposes them via HTTP for E2E test assertions.
echoAdapter:
enabled: true
image:
repository: ghcr.io/osac-project/metering-echo-adapter
tag: latest
pullPolicy: Always
database:
connection:
- secret:
Expand Down
20 changes: 20 additions & 0 deletions osac-metering/adapters/.golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
version: "2"
run:
timeout: 5m
linters:
enable:
- errcheck
- govet
- ineffassign
- staticcheck
- unused
- misspell
- revive
settings:
revive:
rules:
- name: exported
disabled: true
formatters:
enable:
- goimports
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ RUN go mod download

COPY . ./

RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -buildvcs=false -a -o test-adapter ./cmd/test-adapter
RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -buildvcs=false -a -o echo-adapter ./cmd/echo-adapter

FROM registry.access.redhat.com/ubi10-minimal:10.2
WORKDIR /
COPY --from=builder /opt/app-root/src/test-adapter .
COPY --from=builder /opt/app-root/src/echo-adapter .
USER 65532:65532
ENTRYPOINT ["/test-adapter"]
ENTRYPOINT ["/echo-adapter"]
Comment on lines +15 to +17

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Define an image health check.

The final image has no HEALTHCHECK. Add a health check that verifies the echo adapter health endpoint without adding build tools to the final image.

As per path instructions, Containerfiles must define HEALTHCHECK.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@osac-metering/adapters/Containerfile.echo-adapter` around lines 15 - 17, Add
a HEALTHCHECK to the final image in Containerfile.echo-adapter that probes the
echo adapter’s health endpoint using only utilities already available in the
runtime image, without installing build tools. Keep it alongside the existing
USER and ENTRYPOINT directives and use the adapter’s documented health endpoint
and appropriate failure timing.

Source: Path instructions

31 changes: 31 additions & 0 deletions osac-metering/adapters/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Copyright (c) 2026 Red Hat Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
# in compliance with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0

GINKGO ?= go run github.com/onsi/ginkgo/v2/ginkgo
GOLANGCI_LINT_VERSION ?= v2.12.1

LOCALBIN ?= $(shell pwd)/bin
GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint

.PHONY: build-echo-adapter test lint clean

build-echo-adapter:
@mkdir -p bin
go build -o bin/echo-adapter ./cmd/echo-adapter
Comment thread
coderabbitai[bot] marked this conversation as resolved.

test:
$(GINKGO) run .

lint: $(GOLANGCI_LINT)
$(GOLANGCI_LINT) run .

$(GOLANGCI_LINT):
@mkdir -p $(LOCALBIN)
GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)

clean:
rm -rf bin/
69 changes: 69 additions & 0 deletions osac-metering/adapters/adapter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
Copyright (c) 2026 Red Hat, Inc.

Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
in compliance with the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0
*/

package adapters

import (
"context"

cloudevents "github.com/cloudevents/sdk-go/v2"
)

// MeteringEvent wraps a CloudEvent with its Kafka coordinates.
type MeteringEvent struct {
CloudEvent cloudevents.Event
Topic string
Partition int32
Offset int64
}

// SubmitResult is returned by Flush to report the outcome.
type SubmitResult struct {
ProviderEventID string
Idempotent bool
}

// ProviderAdapter is the interface that concrete provider adapters implement.
// The Runner calls Submit per event and Flush on a configurable interval.
type ProviderAdapter interface {
// Name returns the provider name used as a Prometheus label.
Name() string
// Submit processes a single metering event.
Submit(ctx context.Context, event MeteringEvent) error
// Flush uploads any buffered events. Called on the flush ticker
// (default 10s) and on graceful shutdown.
Flush(ctx context.Context) (SubmitResult, error)
// HealthCheck verifies connectivity to the provider.
HealthCheck(ctx context.Context) error
// Close releases resources after the final Flush.
Close() error
Comment on lines +44 to +45

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Add a context to Close.

Close can block on provider I/O during shutdown. The current contract cannot apply cancellation or a deadline. Update it to accept context.Context, and call it with a bounded shutdown context from Runner.Run.

Proposed interface change
- Close() error
+ Close(ctx context.Context) error

As per path instructions, use "context.Context for cancellation and timeouts."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@osac-metering/adapters/adapter.go` around lines 44 - 45, Update the adapter
interface’s Close method to accept context.Context, then modify Runner.Run to
create and pass a bounded shutdown context when closing the adapter. Propagate
that context through the shutdown path so provider I/O can be cancelled or
time-limited while preserving the existing final Flush-before-Close order.

Source: Path instructions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The current Close() error signature follows io.Closer's interface pattern, which is the standard for resource cleanup. Changing to Close(ctx context.Context) error would prevent implementors from satisfying io.Closer. The adapter's Close() is called after the final Flush(ctx) (which already accepts context), so the window for cleanup work is bounded by the shutdown timeout on the flush. If a concrete adapter's Close() needs a timeout, it can create its own internal context.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@amito, io.Closer compatibility is a valid reason to retain Close() error.

However, the shutdown timeout on Flush(ctx) does not bound the subsequent Close() call. Close() can still block on provider I/O after Flush(ctx) returns.

An internal timeout in each concrete adapter can mitigate this risk. The Runner cannot enforce or cancel that timeout through the current interface. The availability concern therefore remains, but the API compatibility trade-off is clear.


✏️ Learnings added
Learnt from: amito
Repo: osac-project/osac PR: 174
File: osac-metering/adapters/adapter.go:44-45
Timestamp: 2026-08-09T06:40:21.400Z
Learning: In `osac-metering/adapters`, `ProviderAdapter.Close` intentionally uses the `io.Closer`-compatible signature `Close() error`. The runner calls `Close()` after the final context-aware `Flush(ctx)`. Concrete provider adapters must establish their own bounded cleanup context if `Close()` can perform blocking provider I/O.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

}

// RetryableError is an optional marker for documentation purposes. The Runner
// retries all errors by default — only errors wrapped in NonRetryableError are
// skipped. Wrapping in RetryableError makes the intent explicit but does not
// change retry behavior.
type RetryableError struct{ Err error }

func (e *RetryableError) Error() string { return e.Err.Error() }
func (e *RetryableError) Unwrap() error { return e.Err }

// NonRetryableError signals the runner should skip the event without retry.
type NonRetryableError struct{ Err error }

func (e *NonRetryableError) Error() string { return e.Err.Error() }
func (e *NonRetryableError) Unwrap() error { return e.Err }

// KafkaConfig configures the Kafka consumer connection.
type KafkaConfig struct {
TLSEnabled bool // Enable TLS for broker connections
TLSCACert string // Path to CA certificate file (empty = system CAs)
SASLUser string // SASL/SCRAM username
SASLPassFile string // Path to file containing SASL password
}
62 changes: 62 additions & 0 deletions osac-metering/adapters/adapter_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
Copyright (c) 2026 Red Hat, Inc.

Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
in compliance with the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0
*/

package adapters

import (
"errors"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("Error Types", func() {
Describe("RetryableError", func() {
It("wraps and unwraps the underlying error", func() {
underlying := errors.New("connection timeout")
err := &RetryableError{Err: underlying}

Expect(err.Error()).To(Equal("connection timeout"))
Expect(errors.Unwrap(err)).To(Equal(underlying))
})

It("can be detected with errors.As", func() {
underlying := errors.New("temporary failure")
err := &RetryableError{Err: underlying}

var retryable *RetryableError
Expect(errors.As(err, &retryable)).To(BeTrue())
})
})

Describe("NonRetryableError", func() {
It("wraps and unwraps the underlying error", func() {
underlying := errors.New("malformed event")
err := &NonRetryableError{Err: underlying}

Expect(err.Error()).To(Equal("malformed event"))
Expect(errors.Unwrap(err)).To(Equal(underlying))
})

It("can be detected with errors.As", func() {
underlying := errors.New("permanent failure")
err := &NonRetryableError{Err: underlying}

var nonRetryable *NonRetryableError
Expect(errors.As(err, &nonRetryable)).To(BeTrue())
})

It("is not detected as RetryableError", func() {
err := &NonRetryableError{Err: errors.New("fail")}

var retryable *RetryableError
Expect(errors.As(err, &retryable)).To(BeFalse())
})
})
})
22 changes: 22 additions & 0 deletions osac-metering/adapters/adapters_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
Copyright (c) 2026 Red Hat, Inc.

Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
in compliance with the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0
*/

package adapters

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestAdapters(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Adapters Suite")
}
Loading
Loading