goSDN issueshttps://code.fbi.h-da.de/danet/gosdn/-/issues2023-06-16T09:23:57Zhttps://code.fbi.h-da.de/danet/gosdn/-/issues/113CLI and CMD Integration Tests2023-06-16T09:23:57ZGhost UserCLI and CMD Integration Tests## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
cliIntegration_test.go and cmdIntegration_test.go need to be updated to reflect changes introduced by commit-confirm ...## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
cliIntegration_test.go and cmdIntegration_test.go need to be updated to reflect changes introduced by commit-confirm mechanic.
## Area of the system
<!-- This might only be one part, but may involve multiple sections !-->
Integration test
## How does this currently work?
<!-- the current process, and any associated business rules !-->
CLI and CMD integration tests do not reflect the latest changes introduced by commit-confirm mechanic.
## What is the desired way of working?
<!-- after the change, what should the process be, and what should the business rules be !-->
The changes should be tested properly.
## Priority/Severity
<!-- Delete as appropriate. The priority and severity assigned may be different to this !-->
- [x] High (This will bring a huge increase in performance/productivity/usability, or is a legislative requirement)
- [ ] Medium (This will bring a good increase in performance/productivity/usability)
- [ ] Low (anything else e.g., trivial, minor improvements)SORThttps://code.fbi.h-da.de/danet/gosdn/-/issues/116Cancel already running pipeline on MR when pushing new commits2022-04-14T11:24:11ZAndre SterbaCancel already running pipeline on MR when pushing new commitsWhen opening an MR a pipeline run will be triggered.
If a new commit is added to this MR a new pipeline will be started and the pipeline from an earlier commit should be cancelled.
## Problem Statement
Multiple running (parallel) pipeli...When opening an MR a pipeline run will be triggered.
If a new commit is added to this MR a new pipeline will be started and the pipeline from an earlier commit should be cancelled.
## Problem Statement
Multiple running (parallel) pipelines on a single MR.
The result of the "old" pipeline is irrelevant.
## Who will benefit?
Everyone :tada:
## Benefits and risks
This will speed-up MRs and maybe other parallel CI runs and save resources on the test infrastructure.
## Proposed solution
<!-- How would you like to see this issue resolved? !-->
## Examples
There is an GitHub Action ([cancel-workflow-action](https://github.com/styfle/cancel-workflow-action)) that does excactly this.
## Priority/Severity
<!-- Delete as appropriate. The priority and severity assigned may be different to this !-->
- [ ] High (This will bring a huge increase in performance/productivity/usability/legislative cover)
- [ ] Medium (This will bring a good increase in performance/productivity/usability)
- [ ] Low (anything else e.g., trivial, minor improvements)SORThttps://code.fbi.h-da.de/danet/gosdn/-/issues/345AddLink crashes goSDN when Link without correct port config is send.2024-01-11T15:21:05ZNeil-Jocelyn ScharkAddLink crashes goSDN when Link without correct port config is send.A Link contains Ports which contain a Configuration. If it is send without one, goSDN crashes hard.
The AddLink endpoint has validation, but the validation seems to be incorrect/broken/not good enough to catch this.A Link contains Ports which contain a Configuration. If it is send without one, goSDN crashes hard.
The AddLink endpoint has validation, but the validation seems to be incorrect/broken/not good enough to catch this.https://code.fbi.h-da.de/danet/gosdn/-/issues/232Northbound API validation2023-11-23T15:13:53ZAndre SterbaNorthbound API validationCurrently it is very easy to crash the controller by simply not providing a needed field via a NBI request.
E.g. create a new entity and don't provide some configuration field.
We should propably add a gRPC middleware that will check if...Currently it is very easy to crash the controller by simply not providing a needed field via a NBI request.
E.g. create a new entity and don't provide some configuration field.
We should propably add a gRPC middleware that will check if all defined fields in the proto definition are actually there and
reject the request if something is missing.
https://github.com/grpc-ecosystem/go-grpc-middleware/tree/master/validator
+
https://github.com/mwitkow/go-proto-validatorshttps://code.fbi.h-da.de/danet/gosdn/-/issues/152Internal Changes representation is not working as expected2023-06-16T09:23:57ZMalte BauchInternal Changes representation is not working as expectedWhile I've been looking into #145 I've discovered a few more bugs with intended changes ONDs, so called Changes in the goSDN environment.
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you conside...While I've been looking into #145 I've discovered a few more bugs with intended changes ONDs, so called Changes in the goSDN environment.
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
If we consider the following scenario:
1. request a set for /system/config/hostname with a new hostname(a Change is created)
2. commit the requested Change. the commit is accepted and the Change is pushed to the OND (NOTICE: this Change will not be confirmed)
3. request a get for /system/config/hostname (this results in the device.Model being updated with the new hostname)
Now one would expect that after the defined expiration time, the Change is reset, since no confirm of the change has taken place. But the result is: no actual rollback took place.
Since the reference of the previousState of a Change points directly towards device.Model (see [here](https://code.fbi.h-da.de/danet/gosdn/-/blob/develop/nucleus/principalNetworkDomain.go#L316)), it is impossible to detect a difference between previousState and intendedState in this scenario.
This suggests that it would be necessary that previousState should be a reference to a copy of device.Model(instead of a reference to the actual device.Model) at the time of the initialisation of the Change.
However, this problem raises further questions:
1. What happens if there are multiple Changes and if there are confirms in-between?
2. What happens if the device has been modified from somewhere else?
3. ...
**Additional findings:**
- change.Confirm doesn't stop the timer, therefore a reroll will always be initiated
- If device.Model is empty a delete for the path is triggered
- If for a specific Change a reroll is executed, it is not possible to interact further with that Change. Hence, it is not possible in the further course, e.g., to have all Changes displayed. This is because the errChan is blocking, since it has no reciever (see [here](https://code.fbi.h-da.de/danet/gosdn/-/blob/develop/nucleus/change.go#L134)).
- After a COMMITED change has been rerolled, he keeps his state. Instead it should either be reset to PENDING or even be completly removed.v0.1.0 Codename ThreadbareMalte BauchMartin StiemerlingMalte Bauchhttps://code.fbi.h-da.de/danet/gosdn/-/issues/359Update gRPC abstraction API with missing calls and refactoring2024-03-13T09:07:46ZFabian SeidlUpdate gRPC abstraction API with missing calls and refactoring<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
## Expected Behavior
<!--- Tell us what should h...<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
## Expected Behavior
<!--- Tell us what should happen -->
## Actual Behavior
<!--- Tell us what happens instead -->
## Possible Fix
<!--- Not obligatory, but suggest a fix or reason for the bug -->
## Steps to Reproduce
<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug. Include code to reproduce, if relevant -->
1.
2.
3.
4.
## Context
<!--- How has this bug affected you? What were you trying to accomplish? -->
## Your Environment
<!--- Include as many relevant details about the environment you experienced the bug in -->
* Version used:
* Environment name and version (e.g. go v1.16.3 on FreeBSD 13.0-current):
* Server type and version:
* Operating System and version:https://code.fbi.h-da.de/danet/gosdn/-/issues/357Change the way user credentials for authz are provided to not be in context2024-03-19T14:09:03ZFabian SeidlChange the way user credentials for authz are provided to not be in context<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
Currently, the credentials to authenticate/autho...<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
Currently, the credentials to authenticate/authorize are send in the context. This works well for go but not if the communication via gRPC/REST is done from an application written in another language. These apps would not be able to get authorized to do anything in any way. Therefore, the credentials should be provided in another way.
Ideas welcome!
This change will introduce lots of changes in other areas, to be more precise, possibly for our own applications, the tests, the gRPC API abstraction thing and gosdnc. So, implementing a new way of providing the credentials will lead to many small adjustments. Might take some time to find them all.https://code.fbi.h-da.de/danet/gosdn/-/issues/356Credentials returned after registering an app are hard coded, which causes pr...2024-02-22T10:40:33ZFabian SeidlCredentials returned after registering an app are hard coded, which causes problems if RabbitMQ is not hosted on localhost<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
The method controller/app/Service.go/createNewApp...<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
The method controller/app/Service.go/createNewApp always returns "amqp://guest:guest@127.0.0.1:5672" as credentials for an App registering to the Event System. This needs to be fixed.
## Expected Behavior
<!--- Tell us what should happen -->
Method returns correct crededentials dynamically.
## Actual Behavior
<!--- Tell us what happens instead -->
Hard coded string that worked until it didn't :shrug:
## Possible Fix
Use data read from the config file and stored in global vars in config package, e.g. config.AMQPHost.https://code.fbi.h-da.de/danet/gosdn/-/issues/340Implement integration tests for RBAC2024-01-10T08:37:35ZFabian SeidlImplement integration tests for RBAC<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
See the planned tests here: https://etherpad.h-d...<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
See the planned tests here: https://etherpad.h-da.de/p/aiIYo5YxXyHBPe-Z4jSAhttps://code.fbi.h-da.de/danet/gosdn/-/issues/339Implement integration tests for network elements2024-01-10T08:37:26ZFabian SeidlImplement integration tests for network elements<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
See the planned tests here: https://etherpad.h-d...<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
See the planned tests here: https://etherpad.h-da.de/p/aiIYo5YxXyHBPe-Z4jSAhttps://code.fbi.h-da.de/danet/gosdn/-/issues/331Segmentation fault through plugin client2023-11-22T11:06:47ZMalte BauchSegmentation fault through plugin client<!--- Provide a general summary of the issue in the Title above -->
A response of grpc calls within methods of DeviceModelClient is nil as soon as an error occured within the call.
## Description
<!--- Provide a more detailed introducti...<!--- Provide a general summary of the issue in the Title above -->
A response of grpc calls within methods of DeviceModelClient is nil as soon as an error occured within the call.
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
In some methods values of the response were directly accessed and returned. This leads to a segmentation fault if the response is nil.
## Expected Behavior
<!--- Tell us what should happen -->
DeviceModelCLient method calls should not crash the controller
## Actual Behavior
<!--- Tell us what happens instead -->
Some method calls lead to a segmentation fault and crashing the controller
## Possible Fix
<!--- Not obligatory, but suggest a fix or reason for the bug -->
Correct error handlingMalte BauchMalte Bauchhttps://code.fbi.h-da.de/danet/gosdn/-/issues/330Failed network element creation leads to the return of false error messages2023-11-22T11:04:15ZMalte BauchFailed network element creation leads to the return of false error messages<!--- Provide a general summary of the issue in the Title above -->
There are multiple occasions of the wrong error usage within `controller/northbound/server/networkelement.go` in the `addMne` method (https://code.fbi.h-da.de/danet/gosd...<!--- Provide a general summary of the issue in the Title above -->
There are multiple occasions of the wrong error usage within `controller/northbound/server/networkelement.go` in the `addMne` method (https://code.fbi.h-da.de/danet/gosdn/-/blob/b406addb4a7965c7f7a0eac769ca72d7f9115802/controller/northbound/server/networkElement.go#L571)
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
One example of a typo:
```
if err != nil {
if pluginRmErr := plugin.Remove(); err != nil {
return uuid.Nil, pluginRmErr
}
return uuid.Nil, err
}
```
which should be:
```
if err != nil {
if pluginRmErr := plugin.Remove(); pluginRmerr != nil {
return uuid.Nil, pluginRmErr
}
return uuid.Nil, err
}
```
## Expected Behavior
<!--- Tell us what should happen -->
The correct error should be returned
## Actual Behavior
<!--- Tell us what happens instead -->
Wrong errors are returned and therefore sometimes a nil error is returned.
## Possible Fix
<!--- Not obligatory, but suggest a fix or reason for the bug -->
Typos like the one I've mentioned above should be addressed. But in general I think a better way would be to do the clean up of a plugin within a `defer` function.
## Steps to Reproduce
<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug. Include code to reproduce, if relevant -->
1.
2.
3.
4.
## Context
<!--- How has this bug affected you? What were you trying to accomplish? -->
## Your Environment
<!--- Include as many relevant details about the environment you experienced the bug in -->
* Version used:
* Environment name and version (e.g. go v1.16.3 on FreeBSD 13.0-current):
* Server type and version:
* Operating System and version:Malte BauchMalte Bauchhttps://code.fbi.h-da.de/danet/gosdn/-/issues/280Change how gNMI paths are requested via NBI and controller2023-06-14T09:36:38ZFabian SeidlChange how gNMI paths are requested via NBI and controller<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
The current way of handling path requests does no...<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
The current way of handling path requests does not fit our desired behaviour. Currently, when a path is requested via NBI the controller forwards the request, updates the storage with the answer from the target and then forwards the answer to the originally requesting entity. What we actually want is a mix between intended state in the storage and the actual state of the device. This way, it is possible to compare intended and current state and deal with issues with they differ.
This requires a new type of path request via NBI, sth like `GetPathIntended` in addition to the current path requests. The new one should return the intended state from the storage. The original, currently used path request should still be forwarded to the device, but the storage should not be updated with its answer. Instead, the answer would just need to get forwarded to the originally requesting entity. Only once, during the creation of a managed network element in the storage, its answer (but only the writable/configurable paths) should be stored as the model.
- [x] add new path request for intended state
- [x] update original path path request
- [x] update network element creation to fit these changeshttps://code.fbi.h-da.de/danet/gosdn/-/issues/346Discuss future of topology2024-01-22T09:15:34ZNeil-Jocelyn ScharkDiscuss future of topologyCurrently the topology implementation has at least two major bugs. As we currently don't really use it anyway and the general implementation was very specific for a singular problem, we need to discuss the future of the topology. We will...Currently the topology implementation has at least two major bugs. As we currently don't really use it anyway and the general implementation was very specific for a singular problem, we need to discuss the future of the topology. We will need one, but the path forward is not clear. We could fix the current implementation and potentially add more features to it or think of a new one.Neil-Jocelyn ScharkNeil-Jocelyn Scharkhttps://code.fbi.h-da.de/danet/gosdn/-/issues/342Implement integration tests for topology2024-02-22T10:42:00ZNeil-Jocelyn ScharkImplement integration tests for topology<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
## Expected Behavior
<!--- Tell us what should h...<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
## Expected Behavior
<!--- Tell us what should happen -->
## Actual Behavior
<!--- Tell us what happens instead -->
## Possible Fix
<!--- Not obligatory, but suggest a fix or reason for the bug -->
## Steps to Reproduce
<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug. Include code to reproduce, if relevant -->
1.
2.
3.
4.
## Context
<!--- How has this bug affected you? What were you trying to accomplish? -->
## Your Environment
<!--- Include as many relevant details about the environment you experienced the bug in -->
* Version used:
* Environment name and version (e.g. go v1.16.3 on FreeBSD 13.0-current):
* Server type and version:
* Operating System and version:Neil-Jocelyn ScharkNeil-Jocelyn Scharkhttps://code.fbi.h-da.de/danet/gosdn/-/issues/338Check permissions for directoy and file creations from within our code2023-12-04T07:48:54ZFabian SeidlCheck permissions for directoy and file creations from within our code<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
In a recet merge request, there was a notificati...<!--- Provide a general summary of the issue in the Title above -->
## Description
<!--- Provide a more detailed introduction to the issue itself, and why you consider it to be a bug -->
In a recet merge request, there was a notification about a (possible) weakness regarding permissions for directories created from within a running program. We should investigate [this specific case](https://code.fbi.h-da.de/danet/gosdn/-/blob/a05dfd0fe38176f4f4fc245df41168e762e432d1/controller/cmd/root.go#L155) and also look for others and see if we can limit the permissions during all these creation processes.
Please investigate all occurrences of dir/file creationhttps://code.fbi.h-da.de/danet/gosdn/-/issues/335Investigate GitLab-CI services don't reach each other2024-03-04T13:31:48ZNeil-Jocelyn ScharkInvestigate GitLab-CI services don't reach each otherAccording to the docs, services should be able to communicate with each other, if docker networks for each job are activated. But @hda11597 and me @neil.schark did not get it to work. So I implemented a workaround, so that our tests stil...According to the docs, services should be able to communicate with each other, if docker networks for each job are activated. But @hda11597 and me @neil.schark did not get it to work. So I implemented a workaround, so that our tests still run.
But it would be better to get the services running correctly, as it would mean a better test set up.
https://docs.gitlab.com/ee/ci/services/#connecting-services
https://code.fbi.h-da.de/danet/minimal-setup-docker-gitlab-ci
https://gitlab.com/gitlab-org/gitlab-runner/-/issues/1042
Typical pipeline where it didn't work:
https://code.fbi.h-da.de/danet/gosdn/-/jobs/783870
Pipeline where it ran once for whatever reason (never before and never since):
https://code.fbi.h-da.de/danet/gosdn/-/jobs/783590https://code.fbi.h-da.de/danet/gosdn/-/issues/332GetMongoConnection() doens't return error in failure state2023-11-23T14:18:27ZNeil-Jocelyn ScharkGetMongoConnection() doens't return error in failure stateFile `code.fbi.h-da.de/danet/gosdn/controller/nucleus/database/mongo-connection.go`.
Function `GetMongoConnection()` doesn't return an error if one is found, it just prints it.
Proposed solution:
```golang
// GetMongoConnection Retriev...File `code.fbi.h-da.de/danet/gosdn/controller/nucleus/database/mongo-connection.go`.
Function `GetMongoConnection()` doesn't return an error if one is found, it just prints it.
Proposed solution:
```golang
// GetMongoConnection Retrieves a client to the MongoDB.
func GetMongoConnection() (*mongo.Client, context.Context, context.CancelFunc, error) {
mongoConnection := config.DatabaseConnection
ctx, cancel := context.WithTimeout(context.Background(), connectTimeout*time.Second)
client, err := mongo.Connect(ctx, options.Client().ApplyURI(mongoConnection))
if err != nil {
log.Printf("Failed to create client: %v", err)
return client, ctx, cancel, err
}
// Force a connection to verify our connection string
err = client.Ping(ctx, nil)
if err != nil {
log.Printf("Failed to connect to database: %v\n", err)
return client, ctx, cancel, err
}
return client, ctx, cancel, nil
}
```
This leads to a lot of changes in storage code.Malte BauchNeil-Jocelyn ScharkMalte Bauchhttps://code.fbi.h-da.de/danet/gosdn/-/issues/329Create new test concept and set-up2023-12-15T12:50:51ZNeil-Jocelyn ScharkCreate new test concept and set-upNotes added below as unsorted commentsNotes added below as unsorted commentsNeil-Jocelyn ScharkNeil-Jocelyn Scharkhttps://code.fbi.h-da.de/danet/gosdn/-/issues/325Make containerlab runnable on ARM-Macs2023-10-30T10:10:04ZGhost UserMake containerlab runnable on ARM-MacsCurrently users of Macs in general and specifically ARM-Macs have to use SSH to a x86 Linux Server to really develop.
We are looking for a solution that is locally available and performant enough to work with.Currently users of Macs in general and specifically ARM-Macs have to use SSH to a x86 Linux Server to really develop.
We are looking for a solution that is locally available and performant enough to work with.