Skip to content

Commit

Permalink
fix: avoid id misconfiguration
Browse files Browse the repository at this point in the history
  • Loading branch information
davidebianchi committed Nov 12, 2024
1 parent d5f85ef commit bc313f8
Show file tree
Hide file tree
Showing 25 changed files with 94 additions and 95 deletions.
2 changes: 2 additions & 0 deletions docs/10_overview.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Integration Connector Agent

22 changes: 22 additions & 0 deletions docs/20_install.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# How to install

To deploy the service, you need to configure the environment variables and a configuration.

## Environment Variables

The following environment variables are required to run the application.

| Variable | Required | Default | Description |
|-------------------------|----------|---------|--------------------------|
| LOG_LEVEL || info | Log level |
| HTTP_PORT || 8080 | HTTP port |
| HTTP_ADDRESS || 0.0.0.0 | HTTP address exposed by the server |
| SERVICE_PREFIX || / | Prefix for the service endpoints (except for the status and documentation ones) |
| DELAY_SHUTDOWN_SECONDS || 10 | Delay in seconds before the server shutdown. This could be important to correctly enabling the graceful shutdown in Kubernetes environments |
| CONFIGURATION_PATH || | The path where it is the configuration file |

## Configuration

The configuration is needed to define the service behavior: the service to integrate with and where to store the data.

<!-- TODO: -->
Empty file.
1 change: 1 addition & 0 deletions docs/integrations/20_jira.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Jira
Empty file added docs/writers/10_overview.md
Empty file.
Empty file added docs/writers/20_mongo.md
Empty file.
1 change: 0 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ type Integration struct {
Type string `json:"type"`
Authentication Authentication `json:"authentication"`
Writers []Writer `json:"writers"`
EventIDPath string `json:"eventIdPath"`
}

type Configuration struct {
Expand Down
11 changes: 2 additions & 9 deletions internal/config/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@
}
}
},
"eventIdPath": {
"type": "string"
},
"writers": {
"type": "array",
"items": {
Expand All @@ -43,17 +40,14 @@
},
"outputEvent": {
"type": "object"
},
"idField": {
"type": "string"
}
},
"required": [
"type",
"url",
"outputEvent",
"collection",
"idField"
"uniqueIdKey"
]
}
]
Expand All @@ -62,8 +56,7 @@
},
"required": [
"type",
"writers",
"eventIdPath"
"writers"
]
},
"minItems": 1
Expand Down
4 changes: 1 addition & 3 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ func TestLoadServiceConfiguration(t *testing.T) {
Authentication: Authentication{
Secret: SecretSource("MY_SECRET"),
},
EventIDPath: "issue.id",
Writers: []Writer{
{
Type: "mongo",
Expand Down Expand Up @@ -145,7 +144,6 @@ func getExpectedWriterConfig(t *testing.T) string {
"summary": "{{ issue.fields.summary }}",
"createdAt": "{{ issue.fields.created }}",
"description": "{{ issue.fields.description }}"
},
"idField": "key"
}
}`
}
1 change: 0 additions & 1 deletion internal/config/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ func TestWriterConfig(t *testing.T) {
"createdAt": "{{ issue.fields.created }}",
"description": "{{ issue.fields.description }}",
},
IDField: "key",
}, mongoConfig)
})
}
4 changes: 1 addition & 3 deletions internal/config/testdata/all-writer-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
"fromFile": "testdata/secret"
}
},
"eventIdPath": "issue.id",
"writers": [
{
"type": "mongo",
Expand All @@ -20,8 +19,7 @@
"summary": "{{ issue.fields.summary }}",
"createdAt": "{{ issue.fields.created }}",
"description": "{{ issue.fields.description }}"
},
"idField": "key"
}
}
]
}
Expand Down
4 changes: 1 addition & 3 deletions internal/config/testdata/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
"fromFile": "testdata/secret"
}
},
"eventIdPath": "issue.id",
"writers": [
{
"type": "mongo",
Expand All @@ -20,8 +19,7 @@
"summary": "{{ issue.fields.summary }}",
"createdAt": "{{ issue.fields.created }}",
"description": "{{ issue.fields.description }}"
},
"idField": "key"
}
}
]
}
Expand Down
3 changes: 1 addition & 2 deletions internal/integrations/jira/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,5 @@ package jira
// Configuration is the representation of the configuration for a Jira Cloud webhook
type Configuration struct {
// Secret the webhook secret configuration for validating the data received
Secret string
EventIDPath string
Secret string
}
7 changes: 5 additions & 2 deletions internal/integrations/jira/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,15 @@ const (
issueCreated = "jira:issue_created"
issueUpdated = "jira:issue_updated"
issueDeleted = "jira:issue_deleted"

webhookEventFieldPath = "webhookEvent"
eventIDPath = "issue.id"
)

func getPipelineEvent(rawData []byte, eventIDPath string) (entities.PipelineEvent, error) {
func getPipelineEvent(rawData []byte) (entities.PipelineEvent, error) {
parsed := gjson.ParseBytes(rawData)
id := parsed.Get(eventIDPath).String()
webhookEvent := parsed.Get("webhookEvent").String()
webhookEvent := parsed.Get(webhookEventFieldPath).String()

operationType, err := getOperationType(webhookEvent)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/integrations/jira/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func TestEvent(t *testing.T) {

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
event, err := getPipelineEvent([]byte(tc.rawData), "issue.id")
event, err := getPipelineEvent([]byte(tc.rawData))
if tc.expectError != "" {
require.Error(t, err)
require.EqualError(t, err, tc.expectError)
Expand Down
2 changes: 1 addition & 1 deletion internal/integrations/jira/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func webhookHandler(config Configuration, p pipeline.IPipeline[entities.Pipeline
return c.SendStatus(http.StatusOK)
}

event, err := getPipelineEvent(body, config.EventIDPath)
event, err := getPipelineEvent(body)
if err != nil {
log.WithError(err).Error("error unmarshaling event")
return c.Status(http.StatusBadRequest).JSON(utils.ValidationError(err.Error()))
Expand Down
13 changes: 9 additions & 4 deletions internal/integrations/tests/jira_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ func TestJiraIntegration(t *testing.T) {

expectedResults: []map[string]any{
{
"eventId": "12345",
"key": "TEST-123",
"createdAt": "2024-11-06T00:00:00.000Z",
"description": "This is a test issue description",
Expand Down Expand Up @@ -100,6 +101,7 @@ func TestJiraIntegration(t *testing.T) {

expectedResults: []map[string]any{
{
"eventId": "12345",
"key": "TEST-123",
"createdAt": "2024-11-06T00:00:00.000Z",
"description": "This is a test issue description modified",
Expand Down Expand Up @@ -129,12 +131,14 @@ func TestJiraIntegration(t *testing.T) {

expectedResults: []map[string]any{
{
"eventId": "12345",
"key": "TEST-123",
"createdAt": "2024-11-06T00:00:00.000Z",
"description": "This is a test issue description modified",
"summary": "Test modified issue",
},
{
"eventId": "12346",
"key": "TEST-456",
"createdAt": "2024-11-10T00:00:00.000Z",
"description": "This is the second issue",
Expand All @@ -159,10 +163,11 @@ func TestJiraIntegration(t *testing.T) {

expectedResults: []map[string]any{
{
"key": "TEST-456",
"createdAt": "2024-11-10T00:00:00.000Z",
"description": "This is the second issue",
"summary": "Test second issue",
"eventId": "12345",
"key": "TEST-123",
"createdAt": "2024-11-06T00:00:00.000Z",
"description": "This is a test issue description modified",
"summary": "Test modified issue",
},
},
},
Expand Down
4 changes: 1 addition & 3 deletions internal/integrations/tests/testdata/jira/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
"fromFile": "testdata/secret"
}
},
"eventIdPath": "issue.key",
"writers": [
{
"type": "mongo",
Expand All @@ -20,8 +19,7 @@
"summary": "{{ issue.fields.summary }}",
"createdAt": "{{ issue.fields.created }}",
"description": "{{ issue.fields.description }}"
},
"idField": "key"
}
}
]
}
Expand Down
3 changes: 1 addition & 2 deletions internal/server/integrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ func setupIntegrations(ctx context.Context, log *logrus.Logger, cfg *config.Conf
case integration.Jira:
// TODO: improve management of configuration
config := jira.Configuration{
Secret: cfgIntegration.Authentication.Secret.String(),
EventIDPath: cfgIntegration.EventIDPath,
Secret: cfgIntegration.Authentication.Secret.String(),
}

if err := jira.SetupService(ctx, logrus.NewEntry(log), oasRouter, config, writer); err != nil {
Expand Down
12 changes: 4 additions & 8 deletions internal/writer/mongo/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ type Config struct {
Database string `json:"-"`
Collection string `json:"collection"`
OutputEvent map[string]any `json:"outputEvent"`
IDField string `json:"idField"`
}

func (c *Config) Validate() error {
Expand All @@ -40,14 +39,11 @@ func (c *Config) Validate() error {
if c.OutputEvent == nil {
return fmt.Errorf("outputEvent is required")
}
if c.IDField == "" {
return fmt.Errorf("idField is required")
if _, ok := c.OutputEvent["_id"]; ok {
return fmt.Errorf(`outputEvent "_id" field is reserved`)
}
if c.IDField == "_id" {
return fmt.Errorf("idField cannot be \"_id\"")
}
if _, ok := c.OutputEvent[c.IDField]; !ok {
return fmt.Errorf("idField \"%s\" not found in outputEvent", c.IDField)
if _, ok := c.OutputEvent[idField]; ok {
return fmt.Errorf(`outputEvent "%s" field is reserved`, idField)
}

return nil
Expand Down
40 changes: 6 additions & 34 deletions internal/writer/mongo/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,55 +50,27 @@ func TestValidateConfig(t *testing.T) {

expectedError: "outputEvent is required",
},
"throws if IDField not found in output event": {
config: Config{
URL: config.SecretSource("mongodb://localhost:27017"),
Collection: "test",
OutputEvent: map[string]any{},
IDField: "custom_id",
},

expectedError: `idField "custom_id" not found in outputEvent`,
},
"throws if IDField not set": {
config: Config{
URL: config.SecretSource("mongodb://localhost:27017"),
Collection: "test",
OutputEvent: map[string]any{},
},

expectedError: `idField is required`,
},
"set custom IDField": {
"throws if _id is present in OutputEvent": {
config: Config{
URL: config.SecretSource("mongodb://localhost:27017"),
Collection: "test",
OutputEvent: map[string]any{
"custom_id": "my-id",
"_id": "my-id",
},
IDField: "custom_id",
},

expectedConfig: Config{
URL: config.SecretSource("mongodb://localhost:27017"),
Collection: "test",
OutputEvent: map[string]any{
"custom_id": "my-id",
},
IDField: "custom_id",
},
expectedError: `outputEvent "_id" field is reserved`,
},
"_id not supported as IDField": {
"throws if uid is present in OutputEvent": {
config: Config{
URL: config.SecretSource("mongodb://localhost:27017"),
Collection: "test",
OutputEvent: map[string]any{
"_id": "my-id",
"eventId": "my-id",
},
IDField: "_id",
},

expectedError: `idField cannot be "_id"`,
expectedError: `outputEvent "eventId" field is reserved`,
},
}

Expand Down
5 changes: 2 additions & 3 deletions internal/writer/mongo/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ func TestMongo(t *testing.T) {
URL: config.SecretSource(mongoURL),
Database: db,
Collection: collection,
IDField: "key",
})
require.NoError(t, err)

Expand All @@ -52,7 +51,7 @@ func TestMongo(t *testing.T) {
err = w.Write(ctx, e)
require.NoError(t, err)
findAllDocuments(t, coll, []map[string]any{
{"foo": "bar", "key": "123"},
{"eventId": "123", "foo": "bar", "key": "123"},
})
})

Expand All @@ -61,7 +60,7 @@ func TestMongo(t *testing.T) {
err = w.Write(ctx, e)
require.NoError(t, err)
findAllDocuments(t, coll, []map[string]any{
{"foo": "taz", "key": "123", "another": "field"},
{"eventId": "123", "foo": "taz", "key": "123", "another": "field"},
})
})

Expand Down
Loading

0 comments on commit bc313f8

Please sign in to comment.