add provisioners command #252

lekaf974 · 2025-01-24T02:29:21Z

Description

What does this PR do?

Fix #237

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
New chore (expected functionality to be implemented)

Checklist:

My change requires a change to the documentation.
- I have updated the documentation accordingly.
I've signed off with an email address that matches the commit author.

lekaf974 · 2025-01-24T02:44:13Z

@astromechza this is a draft like a starting point. I have exposed type and class on the interface can you take a look and let me know if I am on the good track or if it could have a cleaner way to achieve it before I move forward.
Thanks

also output with current implementation:

go run cmd/score-compose/main.go provisioners list
INFO: Listing provisioners in project 'test1'
INFO: Class: (any), Type: volume
INFO: Class: (any), Type: service-port
INFO: Class: (any), Type: redis
INFO: Class: (any), Type: postgres
INFO: Class: (any), Type: s3
INFO: Class: (any), Type: amqp
INFO: Class: (any), Type: dns
INFO: Class: (any), Type: route
INFO: Class: (any), Type: mongodb
INFO: Class: (any), Type: mysql
INFO: Class: (any), Type: kafka-topic
INFO: Class: (any), Type: elasticsearch

lekaf974 · 2025-01-31T04:52:59Z

Last changes pushed give

go run cmd/score-compose/main.go provisioners list
INFO: Listing provisioners in project 'test1'
+-------+-----------+--------------------------------+
| TYPE  |   CLASS   |            OUTPUTS             |
+-------+-----------+--------------------------------+
| mysql | (any)     | host, port, name, database,    |
|       |           | username, password             |
| mssql | database  | server, port, connection, tcp, |
|       |           | database, username, password   |
| mssql | database  | server, port, connection, tcp, |
|       |           | database, username, password   |
| amqp  | messaging | host, port, vhost, username,   |
|       |           | password                       |
+-------+-----------+--------------------------------+

lekaf974 · 2025-01-31T04:54:15Z

@astromechza could you have a look and tell me if I am on the good track or if you see a more elegant way to extract outputs value ?

lekaf974 · 2025-01-31T04:56:12Z

also all tests are ok

matthieu@pop-os:~/Repos/github/score/score-compose$ go test ./...
?       github.com/score-spec/score-compose/cmd/score-compose   [no test files]
?       github.com/score-spec/score-compose/internal/logging    [no test files]
?       github.com/score-spec/score-compose/internal/project    [no test files]
ok      github.com/score-spec/score-compose/internal/command    3.351s
ok      github.com/score-spec/score-compose/internal/compose    (cached)
ok      github.com/score-spec/score-compose/internal/provisioners       (cached)
ok      github.com/score-spec/score-compose/internal/provisioners/cmdprov       (cached)
ok      github.com/score-spec/score-compose/internal/provisioners/envprov       (cached)
ok      github.com/score-spec/score-compose/internal/provisioners/loader        (cached)
ok      github.com/score-spec/score-compose/internal/provisioners/templateprov  (cached)
ok      github.com/score-spec/score-compose/internal/util       (cached)
ok      github.com/score-spec/score-compose/internal/version    (cached)

lekaf974 · 2025-02-01T03:13:48Z

With latest changes I have params also

+--------------+-----------+------------------+--------------------------------+
|     TYPE     |   CLASS   |      PARAMS      |            OUTPUTS             |
+--------------+-----------+------------------+--------------------------------+
| mysql        | (any)     |                  | host, port, name, database,    |
|              |           |                  | username, password             |
| mssql        | database  |                  | server, port, connection, tcp, |
|              |           |                  | database, username, password   |
| route        | (any)     | port, host, path |                                |
| mssql        | database  |                  | server, port, connection, tcp, |
|              |           |                  | database, username, password   |
| amqp         | messaging |                  | host, port, vhost, username,   |
|              |           |                  | password                       |
| service-port | (any)     | workload, port   | hostname, port                 |
+--------------+-----------+------------------+--------------------------------+

delca85

Changes look very good, I'd propose to add some unit tests for the more complex provisioner to check their behaviour in isolation.

internal/provisioners/templateprov/template.go

lekaf974 · 2025-02-04T23:23:03Z

thanks @delca85 I'll add more tests on both

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

…s to interface Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

internal/command/provisioners.go

delca85

Thank you @lekaf974 for adding the tests, I've just one more question than I think this is good to go 🚀

mathieu-benoit · 2025-02-05T13:47:32Z

Another thoughts, @lekaf974, I'm wondering if sorting alpabetically both the type and the params/outputs would improve the UX and how users will read/consume this output?

lekaf974 · 2025-02-05T15:56:05Z

Another thoughts, @lekaf974, I'm wondering if sorting alpabetically both the type and the params/outputs would improve the UX and how users will read/consume this output?

Perfect will see how I can do it

internal/command/fixtures/provisioners.custom.golden

internal/command/provisioners.go

go.mod

internal/command/testdata/provisioners.list.valid.golden

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

… order Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

lekaf974 · 2025-02-06T03:37:53Z

lastest output after applied changes requested

~$ score-compose provisioners list
+---------------+-------+------------------+--------------------------------+
|     TYPE      | CLASS |      PARAMS      |            OUTPUTS             |
+---------------+-------+------------------+--------------------------------+
| amqp          | (any) |                  | host, password, port,          |
|               |       |                  | username, vhost                |
+---------------+-------+------------------+--------------------------------+
| dns           | (any) |                  | host                           |
+---------------+-------+------------------+--------------------------------+
| elasticsearch | (any) |                  | host, password, port, username |
+---------------+-------+------------------+--------------------------------+
| kafka-topic   | (any) |                  | host, name, num_partitions,    |
|               |       |                  | port                           |
+---------------+-------+------------------+--------------------------------+
| mongodb       | (any) |                  | connection, host, mongodb,     |
|               |       |                  | password, port, username       |
+---------------+-------+------------------+--------------------------------+
| mysql         | (any) |                  | database, host, name,          |
|               |       |                  | password, port, username       |
+---------------+-------+------------------+--------------------------------+
| postgres      | (any) |                  | database, host, name,          |
|               |       |                  | password, port, username       |
+---------------+-------+------------------+--------------------------------+
| redis         | (any) |                  | host, password, port, username |
+---------------+-------+------------------+--------------------------------+
| route         | (any) | host, path, port |                                |
+---------------+-------+------------------+--------------------------------+
| s3            | (any) |                  | access_key_id,                 |
|               |       |                  | aws_access_key_id,             |
|               |       |                  | aws_secret_key, bucket,        |
|               |       |                  | endpoint, http, region,        |
|               |       |                  | secret_key                     |
+---------------+-------+------------------+--------------------------------+
| service-port  | (any) | port, workload   | hostname, port                 |
+---------------+-------+------------------+--------------------------------+
| volume        | (any) |                  | source, type                   |
+---------------+-------+------------------+--------------------------------+

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

mathieu-benoit · 2025-02-06T12:27:36Z

@lekaf974 @astromechza, in addition to this default table format (awesome!) and out of curiosity, -o json could be very interesting to more easily process this output, but assuming that it's out of scope of this PR for now and that we will create a future issue/PR for this? Or do we want to tackle this here?

lekaf974 · 2025-02-06T13:41:07Z

@lekaf974 @astromechza, in addition to this default table format (awesome!) and out of curiosity, -o json could be very interesting to more easily process this output, but assuming that it's out of scope of this PR for now and that we will create a future issue/PR for this? Or do we want to tackle this here?

Perhaps we could do it in a second Ticket ? But I could add it here, it looks not a big work since we have already the data

mathieu-benoit · 2025-02-06T14:14:13Z

Perhaps we could do it in a second Ticket ?

Yes for sure, totally fine with that. Up to you ;)

delca85

LGTM! Thank you very much for your work @lekaf974 !

lekaf974 · 2025-02-07T19:08:36Z

LGTM! Thank you very much for your work @lekaf974 !

I am happy to contribute, i am trying to improve my development skills in Golang so just give it a try!

lekaf974 · 2025-02-10T23:34:46Z

Any updates ?

mathieu-benoit · 2025-02-11T02:04:16Z

@astromechza, one last review on this one please? Thanks!

astromechza

Sorry I've been slow, been on holiday, but getting back now!

astromechza · 2025-02-15T09:46:47Z

internal/provisioners/templateprov/template.go

+		return []string{}
+	}
+	re := regexp.MustCompile(`\w+:`)
+	matches := re.FindAllString(p.OutputsTemplate, -1)


I'm sorry I've been slow getting back to this, but this has flaws and is an NP-hard problem in general (you can't know the result without evaluating the template). The Params() function above has the same issue.

Please just hardcode the expected outputs and supported parameters as fields in the provisioner, and backfill the existing default provisioners to have the correct values.

This makes it simpler, and means we can enforce the values elsewhere in score-compose (probably in a separate ticket):

During provisioner execution, we can validate that the parameters passed in are all in the Params() list of the provisioner.

After provisioner execution, we can validate that the outputs returned from the driver are all in the Outputs() list.

I'll take a look during next week on your point, thanks for feedback

Cool, sorry for the run-around on this :)

lekaf974 marked this pull request as ready for review February 1, 2025 15:01

delca85 reviewed Feb 4, 2025

View reviewed changes

internal/provisioners/templateprov/template.go Outdated Show resolved Hide resolved

lekaf974 added 10 commits February 4, 2025 21:32

feat: add provisioners list command

3accb37

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

feat(cli): First iteration to list provisionners adding type and clas…

fd46750

…s to interface Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

fix: return all custom providers files

4cc139a

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

chore: move provisioners listing in dedicated functiom

db45ea3

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

chore: fix test output

c077fa4

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

feat: add table display

dee1dfc

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

add outputs to displayed array

1d1c179

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

add tests for provionners command

05e9429

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

chore: remove unnecessary comments

762e1ef

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

chore: update getProvionersFiles function

10d5089

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

lekaf974 force-pushed the feat/add-provisioners-cmd branch from 66d84af to 10d5089 Compare February 5, 2025 02:33

test: increase test coverage after adding provisioners list command

c39326f

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

lekaf974 requested a review from delca85 February 5, 2025 03:29

delca85 reviewed Feb 5, 2025

View reviewed changes

internal/command/provisioners.go Outdated Show resolved Hide resolved

delca85 reviewed Feb 5, 2025

View reviewed changes

astromechza requested changes Feb 5, 2025

View reviewed changes

lekaf974 added 4 commits February 5, 2025 20:30

chore: use LoadProvisionersFromDirectory + enhance table output

55953b7

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

chore: sort params and output by alphabetic order

16ed292

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

chore: update golden resut after sort params and output by alphabetic…

59faa48

… order Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

chore: add sort provisioners list by type

2bc0b92

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

chore: update unit test and fix sorting issue on provisionners list

d16316a

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

test: enhance test suite

b95f63d

Signed-off-by: lekaf974 <matthieu.evrin@gmail.com>

lekaf974 requested review from astromechza and delca85 February 6, 2025 14:36

delca85 approved these changes Feb 7, 2025

View reviewed changes

astromechza requested changes Feb 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add provisioners command #252

add provisioners command #252

lekaf974 commented Jan 24, 2025 •

edited

Loading

lekaf974 commented Jan 24, 2025 •

edited

Loading

lekaf974 commented Jan 31, 2025

lekaf974 commented Jan 31, 2025

lekaf974 commented Jan 31, 2025

lekaf974 commented Feb 1, 2025

delca85 left a comment

lekaf974 commented Feb 4, 2025

delca85 left a comment

mathieu-benoit commented Feb 5, 2025

lekaf974 commented Feb 5, 2025

lekaf974 commented Feb 6, 2025 •

edited

Loading

mathieu-benoit commented Feb 6, 2025 •

edited

Loading

lekaf974 commented Feb 6, 2025

mathieu-benoit commented Feb 6, 2025 •

edited

Loading

delca85 left a comment

lekaf974 commented Feb 7, 2025

lekaf974 commented Feb 10, 2025

mathieu-benoit commented Feb 11, 2025

astromechza left a comment

astromechza Feb 15, 2025

lekaf974 Feb 15, 2025

astromechza Feb 17, 2025

add provisioners command #252

Are you sure you want to change the base?

add provisioners command #252

Conversation

lekaf974 commented Jan 24, 2025 • edited Loading

Description

What does this PR do?

Types of changes

Checklist:

lekaf974 commented Jan 24, 2025 • edited Loading

lekaf974 commented Jan 31, 2025

lekaf974 commented Jan 31, 2025

lekaf974 commented Jan 31, 2025

lekaf974 commented Feb 1, 2025

delca85 left a comment

Choose a reason for hiding this comment

lekaf974 commented Feb 4, 2025

delca85 left a comment

Choose a reason for hiding this comment

mathieu-benoit commented Feb 5, 2025

lekaf974 commented Feb 5, 2025

lekaf974 commented Feb 6, 2025 • edited Loading

mathieu-benoit commented Feb 6, 2025 • edited Loading

lekaf974 commented Feb 6, 2025

mathieu-benoit commented Feb 6, 2025 • edited Loading

delca85 left a comment

Choose a reason for hiding this comment

lekaf974 commented Feb 7, 2025

lekaf974 commented Feb 10, 2025

mathieu-benoit commented Feb 11, 2025

astromechza left a comment

Choose a reason for hiding this comment

astromechza Feb 15, 2025

Choose a reason for hiding this comment

lekaf974 Feb 15, 2025

Choose a reason for hiding this comment

astromechza Feb 17, 2025

Choose a reason for hiding this comment

lekaf974 commented Jan 24, 2025 •

edited

Loading

lekaf974 commented Jan 24, 2025 •

edited

Loading

lekaf974 commented Feb 6, 2025 •

edited

Loading

mathieu-benoit commented Feb 6, 2025 •

edited

Loading

mathieu-benoit commented Feb 6, 2025 •

edited

Loading