From 368d8b89130b295f0f6a05347d1e242a9a8adc29 Mon Sep 17 00:00:00 2001 From: Anthony Casagrande Date: Tue, 2 May 2023 12:13:43 -0700 Subject: [PATCH 1/3] feat: implement secret change support, refactor cred mgmt Signed-off-by: Anthony Casagrande --- cmd/main.go | 3 +- go.mod | 22 +- go.sum | 57 ++--- .../driver/basenotificationresthandler.go | 15 +- internal/driver/checkstatuses.go | 34 +-- internal/driver/checkstatuses_test.go | 2 - internal/driver/credentials.go | 54 ++++- internal/driver/credentials_test.go | 61 ++--- internal/driver/driver.go | 208 +++++------------- internal/driver/driver_test.go | 7 +- internal/driver/macmapper.go | 7 +- internal/driver/macmapper_test.go | 2 - internal/driver/onvifclient.go | 115 +++++++++- internal/driver/onvifdiscovery.go | 13 +- internal/driver/pullpointsubscriber.go | 2 +- 15 files changed, 336 insertions(+), 266 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index ff92bbc7..44bbdf47 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -18,6 +18,5 @@ const ( ) func main() { - sd := driver.Driver{} - startup.Bootstrap(serviceName, device_camera.Version, &sd) + startup.Bootstrap(serviceName, device_camera.Version, driver.NewDriver()) } diff --git a/go.mod b/go.mod index 6597542b..8a5e5a49 100644 --- a/go.mod +++ b/go.mod @@ -4,9 +4,9 @@ go 1.20 require ( github.com/IOTechSystems/onvif v0.1.6 - github.com/edgexfoundry/device-sdk-go/v3 v3.0.0-dev.75 - github.com/edgexfoundry/go-mod-bootstrap/v3 v3.0.0-dev.71 - github.com/edgexfoundry/go-mod-core-contracts/v3 v3.0.0-dev.37 + github.com/edgexfoundry/device-sdk-go/v3 v3.0.0-dev.77 + github.com/edgexfoundry/go-mod-bootstrap/v3 v3.0.0-dev.78 + github.com/edgexfoundry/go-mod-core-contracts/v3 v3.0.0-dev.39 github.com/google/uuid v1.3.0 github.com/gorilla/mux v1.8.0 github.com/stretchr/testify v1.8.2 @@ -22,9 +22,9 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/eclipse/paho.mqtt.golang v1.4.2 // indirect github.com/edgexfoundry/go-mod-configuration/v3 v3.0.0-dev.10 // indirect - github.com/edgexfoundry/go-mod-messaging/v3 v3.0.0-dev.25 // indirect + github.com/edgexfoundry/go-mod-messaging/v3 v3.0.0-dev.28 // indirect github.com/edgexfoundry/go-mod-registry/v3 v3.0.0-dev.7 // indirect - github.com/edgexfoundry/go-mod-secrets/v3 v3.0.0-dev.13 // indirect + github.com/edgexfoundry/go-mod-secrets/v3 v3.0.0-dev.14 // indirect github.com/elgs/gostrgen v0.0.0-20161222160715-9d61ae07eeae // indirect github.com/fatih/color v1.9.0 // indirect github.com/fxamacker/cbor/v2 v2.4.0 // indirect @@ -33,9 +33,10 @@ require ( github.com/go-logfmt/logfmt v0.5.1 // indirect github.com/go-playground/locales v0.14.1 // indirect github.com/go-playground/universal-translator v0.18.1 // indirect - github.com/go-playground/validator/v10 v10.12.0 // indirect + github.com/go-playground/validator/v10 v10.13.0 // indirect github.com/go-redis/redis/v7 v7.3.0 // indirect github.com/golang/protobuf v1.5.2 // indirect + github.com/google/btree v1.0.0 // indirect github.com/gorilla/websocket v1.4.2 // indirect github.com/hashicorp/consul/api v1.20.0 // indirect github.com/hashicorp/errwrap v1.0.0 // indirect @@ -46,15 +47,16 @@ require ( github.com/hashicorp/go-rootcerts v1.0.2 // indirect github.com/hashicorp/golang-lru v0.5.4 // indirect github.com/hashicorp/serf v0.10.1 // indirect - github.com/leodido/go-urn v1.2.2 // indirect - github.com/mattn/go-colorable v0.1.12 // indirect - github.com/mattn/go-isatty v0.0.14 // indirect + github.com/kr/pretty v0.3.1 // indirect + github.com/leodido/go-urn v1.2.3 // indirect + github.com/mattn/go-colorable v0.1.13 // indirect + github.com/mattn/go-isatty v0.0.18 // indirect github.com/mitchellh/consulstructure v0.0.0-20190329231841-56fdc4d2da54 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect github.com/mitchellh/go-homedir v1.1.0 // indirect github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/mitchellh/reflectwalk v1.0.2 // indirect - github.com/nats-io/nats.go v1.24.0 // indirect + github.com/nats-io/nats.go v1.25.0 // indirect github.com/nats-io/nkeys v0.4.4 // indirect github.com/nats-io/nuid v1.0.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect diff --git a/go.sum b/go.sum index 85eb2cf7..1980c9ce 100644 --- a/go.sum +++ b/go.sum @@ -28,25 +28,26 @@ github.com/circonus-labs/circonus-gometrics v2.3.1+incompatible/go.mod h1:nmEj6D github.com/circonus-labs/circonusllhist v0.1.3/go.mod h1:kMXHVDlOchFAehlya5ePtbp5jckzBHf4XRpQvBOLI+I= github.com/clbanning/mxj/v2 v2.3.2 h1:DSkU65zfrBHtrggxd54X9pK1z/Lw2OwSW5D8p+x1toE= github.com/clbanning/mxj/v2 v2.3.2/go.mod h1:hNiWqW14h+kc+MdF9C6/YoRfjEJoR3ou6tn/Qo+ve2s= +github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/eclipse/paho.mqtt.golang v1.4.2 h1:66wOzfUHSSI1zamx7jR6yMEI5EuHnT1G6rNA5PM12m4= github.com/eclipse/paho.mqtt.golang v1.4.2/go.mod h1:JGt0RsEwEX+Xa/agj90YJ9d9DH2b7upDZMK9HRbFvCA= -github.com/edgexfoundry/device-sdk-go/v3 v3.0.0-dev.75 h1:vYAsmqGdC8hrbPO/5/S0vkqoE0a+8Pw0eZDxkrRILMQ= -github.com/edgexfoundry/device-sdk-go/v3 v3.0.0-dev.75/go.mod h1:QMKkUgH17zvPr9BHEMngT5mEewLWPMGg4NHhvBgBiHU= -github.com/edgexfoundry/go-mod-bootstrap/v3 v3.0.0-dev.71 h1:+lMhxjPuvRJ6syy1mzI1wbjfqzJPmkNMBruaiCUcEQQ= -github.com/edgexfoundry/go-mod-bootstrap/v3 v3.0.0-dev.71/go.mod h1:GAoMAkiij8VjRAb9jRw3+skSE/VPDXBqhi5jsGSwIVM= +github.com/edgexfoundry/device-sdk-go/v3 v3.0.0-dev.77 h1:19MjelDoTw08STKmFM8UYN8FCAjy1bdx8FHEVpm539g= +github.com/edgexfoundry/device-sdk-go/v3 v3.0.0-dev.77/go.mod h1:eqYSdDqo/7dNJ3eED50sMmiGCrk6nZNZMD+MJb3qfCk= +github.com/edgexfoundry/go-mod-bootstrap/v3 v3.0.0-dev.78 h1:1cj66uLeAswj01+krlmWP6+k+HSlZeaNRXET8bKf0/I= +github.com/edgexfoundry/go-mod-bootstrap/v3 v3.0.0-dev.78/go.mod h1:oXI84yFlBa+wNwX92tU7hYLaq/XwhUuBgiVGRLzZpMs= github.com/edgexfoundry/go-mod-configuration/v3 v3.0.0-dev.10 h1:iDuAO3vpBQnlQuFhai/NATbJkiYXxo3bPCtSnFl07Yw= github.com/edgexfoundry/go-mod-configuration/v3 v3.0.0-dev.10/go.mod h1:8RlYm5CPzZgUsfXDWVP1TIeUMhsDNIdRdj1HXdomtOI= -github.com/edgexfoundry/go-mod-core-contracts/v3 v3.0.0-dev.37 h1:UTYlgeT2SxW7I/pjVcBOK0uBQGn0GAfdGJQbRU/vaoM= -github.com/edgexfoundry/go-mod-core-contracts/v3 v3.0.0-dev.37/go.mod h1:1YS2J5NfPCd7TYBk0alu+RR5EBzBb+bnG0KF1qNRQYY= -github.com/edgexfoundry/go-mod-messaging/v3 v3.0.0-dev.25 h1:IElkfO4qZt7+VOmQnsLNB3cDsm5Kfe6cYo/S1RU/2HA= -github.com/edgexfoundry/go-mod-messaging/v3 v3.0.0-dev.25/go.mod h1:04phI5qPCFC2bfG1xZ5h6aiGH8Zp8gKDEljHjo/B+Fs= +github.com/edgexfoundry/go-mod-core-contracts/v3 v3.0.0-dev.39 h1:tlw1Bd/hq4D6PQFo3BaN0JUkSqDuAPduaGBYCvoEYTg= +github.com/edgexfoundry/go-mod-core-contracts/v3 v3.0.0-dev.39/go.mod h1:zzzWGWij6wAqm1go9TLs++TFMIsBqBb1eRnIj4mRxGw= +github.com/edgexfoundry/go-mod-messaging/v3 v3.0.0-dev.28 h1:7i64Llcg8xRS/kuMDRPcBP0NuKS3tbeiGm0Rz1kUrco= +github.com/edgexfoundry/go-mod-messaging/v3 v3.0.0-dev.28/go.mod h1:/hUUM2+h8ajcQlNuyxCM6PeDE1Tz1S/EfvR4TmJAja8= github.com/edgexfoundry/go-mod-registry/v3 v3.0.0-dev.7 h1:sje0agoLi8ayEFxGO3xtN7P/IXwjZUUxpC8G2fCTu44= github.com/edgexfoundry/go-mod-registry/v3 v3.0.0-dev.7/go.mod h1:SGyo4fAHzOhDAd2Usa9RaBT/sOzkbceIqLrDG0+iYy8= -github.com/edgexfoundry/go-mod-secrets/v3 v3.0.0-dev.13 h1:FAsSIwsGYg+sxCV5eeJz05MgWT7t864CmCArue8vRJw= -github.com/edgexfoundry/go-mod-secrets/v3 v3.0.0-dev.13/go.mod h1:ZLrhUAvVafUoIsiIlYP3ibIQELCdEgAdUmTXY8CIUv8= +github.com/edgexfoundry/go-mod-secrets/v3 v3.0.0-dev.14 h1:2jG1CBKBXPQdkM8AN1PweaTIaCgudZ6tua/BBBHgq+A= +github.com/edgexfoundry/go-mod-secrets/v3 v3.0.0-dev.14/go.mod h1:FRKg6x/tVpYHfy/aMy5flTxxw3+YeYaQds8csCUqNTE= github.com/elgs/gostrgen v0.0.0-20161222160715-9d61ae07eeae h1:3KvK2DmA7TxQ6PZ2f0rWbdqjgJhRcqgbY70bBeE4clI= github.com/elgs/gostrgen v0.0.0-20161222160715-9d61ae07eeae/go.mod h1:wruC5r2gHdr/JIUs5Rr1V45YtsAzKXZxAnn/5rPC97g= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= @@ -76,8 +77,8 @@ github.com/go-playground/universal-translator v0.17.0/go.mod h1:UkSxE5sNxxRwHyU+ github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY= github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY= github.com/go-playground/validator/v10 v10.2.0/go.mod h1:uOYAAleCW8F/7oMFd6aG0GOhaH6EGOAJShg8Id5JGkI= -github.com/go-playground/validator/v10 v10.12.0 h1:E4gtWgxWxp8YSxExrQFv5BpCahla0PVF2oTTEYaWQGI= -github.com/go-playground/validator/v10 v10.12.0/go.mod h1:hCAPuzYvKdP33pxWa+2+6AIKXEKqjIUyqsNCtbsSJrA= +github.com/go-playground/validator/v10 v10.13.0 h1:cFRQdfaSMCOSfGCCLB20MHvuoHb/s5G8L5pu2ppK5AQ= +github.com/go-playground/validator/v10 v10.13.0/go.mod h1:dwu7+CG8/CtBiJFZDz4e+5Upb6OLw04gtBYw0mcG/z4= github.com/go-redis/redis/v7 v7.3.0 h1:3oHqd0W7f/VLKBxeYTEpqdMUsmMectngjM9OtoRoIgg= github.com/go-redis/redis/v7 v7.3.0/go.mod h1:JDNMw23GTyLNC4GZu9njt15ctBQVn7xjRfnwdHj/Dcg= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= @@ -89,8 +90,9 @@ github.com/golang/protobuf v1.3.3/go.mod h1:vzj43D7+SQXF/4pzW/hwtAqwc6iTitCiVSaW github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.2 h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw= github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= -github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c h1:964Od4U6p2jUkFxvCydnIczKteheJEzHRToSGK3Bnlw= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= +github.com/google/btree v1.0.0 h1:0udJVsspx3VBr5FwtLhQQtuAsVc79tTq0ocGIPAU6qo= +github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= @@ -150,25 +152,28 @@ github.com/klauspost/compress v1.16.4 h1:91KN02FnsOYhuunwU4ssRe8lc2JosWmizWa91B5 github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= -github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII= -github.com/leodido/go-urn v1.2.2 h1:7z68G0FCGvDk646jz1AelTYNYWrTNm0bEcFAo147wt4= -github.com/leodido/go-urn v1.2.2/go.mod h1:kUaIbLZWttglzwNuG0pgsh5vuV6u2YcGBYz1hIPjtOQ= +github.com/leodido/go-urn v1.2.3 h1:6BE2vPT0lqoz3fmOesHZiaiFh7889ssCo2GMvLCfiuA= +github.com/leodido/go-urn v1.2.3/go.mod h1:7ZrI8mTSeBSHl/UaRyKQW1qZeMgak41ANeCNaVckg+4= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.6/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= -github.com/mattn/go-colorable v0.1.12 h1:jF+Du6AlPIjs2BiUiQlKOX0rt3SujHxPnksPKZbaA40= -github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= +github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= +github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/mattn/go-isatty v0.0.10/go.mod h1:qgIWMr58cqv1PHHyhnkY9lrL7etaEgOFcMEpPG5Rm84= github.com/mattn/go-isatty v0.0.11/go.mod h1:PhnuNfih5lzO57/f3n+odYbM4JtupLOxQOAqxQCu2WE= github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= -github.com/mattn/go-isatty v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y= -github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= +github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= +github.com/mattn/go-isatty v0.0.18 h1:DOKFKCQ7FNG2L1rbrmstDN4QVRdS89Nkh85u68Uwp98= +github.com/mattn/go-isatty v0.0.18/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/miekg/dns v1.1.26/go.mod h1:bPDLeHnStXmXAq1m/Ch/hvfNHr14JKNPMBo3VZKjuso= github.com/miekg/dns v1.1.41 h1:WMszZWJG0XmzbK9FEmzH2TVcqYzFesusSIB41b8KHxY= @@ -193,8 +198,8 @@ github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3Rllmb github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/nats-io/jwt/v2 v2.4.1 h1:Y35W1dgbbz2SQUYDPCaclXcuqleVmpbRa7646Jf2EX4= github.com/nats-io/nats-server/v2 v2.9.16 h1:SuNe6AyCcVy0g5326wtyU8TdqYmcPqzTjhkHojAjprc= -github.com/nats-io/nats.go v1.24.0 h1:CRiD8L5GOQu/DcfkmgBcTTIQORMwizF+rPk6T0RaHVQ= -github.com/nats-io/nats.go v1.24.0/go.mod h1:dVQF+BK3SzUZpwyzHedXsvH3EO38aVKuOPkkHlv5hXA= +github.com/nats-io/nats.go v1.25.0 h1:t5/wCPGciR7X3Mu8QOi4jiJaXaWM8qtkLu4lzGZvYHE= +github.com/nats-io/nats.go v1.25.0/go.mod h1:D2WALIhz7V8M0pH8Scx8JZXlg6Oqz5VG+nQkK8nJdvg= github.com/nats-io/nkeys v0.4.4 h1:xvBJ8d69TznjcQl9t6//Q5xXuVhyYiSos6RPtvQNTwA= github.com/nats-io/nkeys v0.4.4/go.mod h1:XUkxdLPTufzlihbamfzQ7mw/VGx6ObUs+0bN5sNvt64= github.com/nats-io/nuid v1.0.1 h1:5iA8DT8V7q8WK2EScv2padNa/rTESc1KdnPw4TC2paw= @@ -207,6 +212,7 @@ github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1Cpa github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/pascaldekloe/goe v0.1.0 h1:cBOtyMzM9HTpWjXfbbunk26uA6nG3a8n06Wieeh0MwY= github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= +github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= @@ -227,7 +233,8 @@ github.com/prometheus/procfs v0.0.2/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsT github.com/prometheus/procfs v0.0.8/go.mod h1:7Qr8sr6344vo1JqZ6HhLceV9o3AJ1Ff+GxbHq6oeK9A= github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 h1:N/ElC8H3+5XpJzTSTfLsJV/mx9Q9g7kxmchpfZyxgzM= github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= -github.com/rwtodd/Go.Sed v0.0.0-20210816025313-55464686f9ef/go.mod h1:8AEUvGVi2uQ5b24BIhcr0GCcpd/RNAFWaN2CJFrWIIQ= +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/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= 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= @@ -303,9 +310,9 @@ golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210303074136-134d130e1a04/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU= golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= diff --git a/internal/driver/basenotificationresthandler.go b/internal/driver/basenotificationresthandler.go index 5a5a885e..9b68a822 100644 --- a/internal/driver/basenotificationresthandler.go +++ b/internal/driver/basenotificationresthandler.go @@ -32,19 +32,16 @@ const ( // RestNotificationHandler handle the notification from the camera and send to async value channel type RestNotificationHandler struct { - sdkService interfaces.DeviceServiceSDK - lc logger.LoggingClient - asyncValues chan<- *models.AsyncValues + sdkService interfaces.DeviceServiceSDK + lc logger.LoggingClient } // NewRestNotificationHandler create a new RestNotificationHandler entity -func NewRestNotificationHandler(service interfaces.DeviceServiceSDK, logger logger.LoggingClient, asyncValues chan<- *models.AsyncValues) *RestNotificationHandler { +func NewRestNotificationHandler(service interfaces.DeviceServiceSDK) *RestNotificationHandler { handler := RestNotificationHandler{ - sdkService: service, - lc: logger, - asyncValues: asyncValues, + sdkService: service, + lc: service.LoggingClient(), } - return &handler } @@ -109,7 +106,7 @@ func (handler RestNotificationHandler) processAsyncRequest(writer http.ResponseW handler.lc.Debugf("Incoming reading received: Device=%s Resource=%s", deviceName, resourceName) - handler.asyncValues <- asyncValues + handler.sdkService.AsyncValuesChannel() <- asyncValues } func (handler RestNotificationHandler) readBody(request *http.Request) ([]byte, error) { diff --git a/internal/driver/checkstatuses.go b/internal/driver/checkstatuses.go index 03b7a3e4..58a8c5a8 100644 --- a/internal/driver/checkstatuses.go +++ b/internal/driver/checkstatuses.go @@ -68,7 +68,7 @@ func (d *Driver) checkStatusOfDevice(device models.Device) { d.lc.Infof("Device %s is now %s, refreshing the device information.", device.Name, UpWithAuth) go func() { // refresh the device information in the background if refreshErr := d.refreshDevice(device); refreshErr != nil { - d.lc.Warnf("An error occurred while refreshing the device %s: %s", + d.lc.Errorf("An error occurred while refreshing the device %s: %s", device.Name, refreshErr.Error()) } }() @@ -82,27 +82,35 @@ func (d *Driver) checkStatusOfDevice(device models.Device) { // Higher degrees of connection are tested first, because if they // succeed, the lower levels of connection will too func (d *Driver) testConnectionMethods(device models.Device) (status string) { - - // sends get capabilities command to device (does not require credentials) - devClient, edgexErr := d.newTemporaryOnvifClient(device) - if edgexErr != nil { - d.lc.Debugf("Connection to %s failed when creating client: %s", device.Name, edgexErr.Message()) - // onvif connection failed, so lets probe it + devClient, err := d.getOnvifClient(device) + if err != nil { + d.lc.Warnf("Error getting onvif client for device %s", device.Name) + // if we do not have a valid onvif client, lets just tcp probe it if d.tcpProbe(device) { return Reachable } return Unreachable + } + // sends GetDeviceInformation command to device (requires authentication) + _, edgexErr := devClient.callOnvifFunction(onvif.DeviceWebService, onvif.GetDeviceInformation, []byte{}) + if edgexErr == nil { + return UpWithAuth // we are authenticated } + d.lc.Debugf("%s command failed for device %s when using authentication: %s", onvif.GetDeviceInformation, device.Name, edgexErr.Message()) - // sends get device information command to device (requires credentials) - _, edgexErr = devClient.callOnvifFunction(onvif.DeviceWebService, onvif.GetDeviceInformation, []byte{}) - if edgexErr != nil { - d.lc.Debugf("%s command failed for device %s when using authentication: %s", onvif.GetDeviceInformation, device.Name, edgexErr.Message()) - return UpWithoutAuth + // sends GetSystemDateAndTime command to device (does not require authentication) + _, edgexErr = devClient.callOnvifFunction(onvif.DeviceWebService, onvif.GetSystemDateAndTime, []byte{}) + if edgexErr == nil { + return UpWithoutAuth // non-authenticated onvif command is working } + d.lc.Debugf("%s command failed for device %s without using authentication: %s", onvif.GetSystemDateAndTime, device.Name, edgexErr.Message()) - return UpWithAuth + // onvif commands are not working, so let us probe it + if d.tcpProbe(device) { + return Reachable + } + return Unreachable } // tcpProbe attempts to make a connection to a specific ip and port list to determine diff --git a/internal/driver/checkstatuses_test.go b/internal/driver/checkstatuses_test.go index 7c485c4b..3efb2c31 100644 --- a/internal/driver/checkstatuses_test.go +++ b/internal/driver/checkstatuses_test.go @@ -11,7 +11,6 @@ import ( "net/http" "net/http/httptest" "strings" - "sync" "testing" "github.com/edgexfoundry/go-mod-core-contracts/v3/models" @@ -56,7 +55,6 @@ func TestUpdateDeviceStatus_noUpdate(t *testing.T) { func TestDriver_TCPProbe(t *testing.T) { driver, _ := createDriverWithMockService() - driver.clientsMu = new(sync.RWMutex) driver.config = &ServiceConfig{ AppCustom: CustomConfig{ ProbeTimeoutMillis: 2000, diff --git a/internal/driver/credentials.go b/internal/driver/credentials.go index 4b7889ba..f11d3fce 100644 --- a/internal/driver/credentials.go +++ b/internal/driver/credentials.go @@ -56,9 +56,9 @@ func IsAuthModeValid(mode string) bool { mode == AuthModeNone } -// tryGetCredentials will attempt one time to get the credentials located at secretName from +// tryGetCredentialsInternal will attempt one time to get the credentials located at secretName from // secret provider and return them, otherwise return an error. -func (d *Driver) tryGetCredentials(secretName string) (Credentials, errors.EdgeX) { +func (d *Driver) tryGetCredentialsInternal(secretName string) (Credentials, errors.EdgeX) { // if the secret name is the special NoAuth magic key, do not look it up, instead return the noAuthCredentials if strings.ToLower(secretName) == noAuthSecretName { return noAuthCredentials, nil @@ -80,6 +80,7 @@ func (d *Driver) tryGetCredentials(secretName string) (Credentials, errors.EdgeX credentials.AuthMode = AuthModeUsernameToken } + d.lc.Tracef("Found credentials from secret name %s", secretName) return credentials, nil } @@ -103,13 +104,56 @@ func (d *Driver) tryGetCredentialsForDevice(device models.Device) (Credentials, d.lc.Warnf("Device %s is missing MAC Address, using default secret name", device.Name) } - credentials, edgexErr := d.tryGetCredentials(secretName) + credentials, edgexErr := d.tryGetCredentialsInternal(secretName) if edgexErr != nil { d.lc.Errorf("Failed to retrieve credentials for the secret name %s: %s", secretName, edgexErr.Error()) return Credentials{}, errors.NewCommonEdgeX(errors.KindServerError, "failed to get credentials", edgexErr) } - d.lc.Debugf("Found credentials from secret name %s for device %s", secretName, device.Name) - return credentials, nil } + +func (d *Driver) secretUpdated(secretName string) { + d.lc.Infof("Secret updated callback called for secretName %s", secretName) + + d.credsCacheMu.Lock() + // remove the cache entry for this secret + delete(d.credsCache, secretName) + d.credsCacheMu.Unlock() + _, _ = d.getCredentials(secretName) // update cached data + + for _, device := range d.sdkService.Devices() { + d.lc.Debugf("Updating onvif client for device %s", device.Name) + err := d.updateOnvifClient(device) + if err != nil { + d.lc.Errorf("Unable to update onvif client for device: %s, %v", device.Name, err) + } + } + + d.lc.Debug("Done updating onvif clients") +} + +func (d *Driver) getCredentials(secretName string) (Credentials, errors.EdgeX) { + d.credsCacheMu.RLock() + creds, found := d.credsCache[secretName] + d.credsCacheMu.RUnlock() + if found { + return creds, nil + } + + d.credsCacheMu.Lock() + creds, err := d.tryGetCredentialsInternal(secretName) + d.credsCacheMu.Unlock() + if err != nil { + return Credentials{}, err + } + // cache the credentials and return them + d.credsCache[secretName] = creds + return creds, nil +} + +func (c Credentials) Equals(other Credentials) bool { + return c.Username == other.Username && + c.Password == other.Password && + c.AuthMode == other.AuthMode +} diff --git a/internal/driver/credentials_test.go b/internal/driver/credentials_test.go index 7a5fcabd..63f362b5 100644 --- a/internal/driver/credentials_test.go +++ b/internal/driver/credentials_test.go @@ -8,7 +8,6 @@ package driver import ( - "sync" "testing" "github.com/IOTechSystems/onvif" @@ -70,9 +69,9 @@ func TestTryGetCredentials(t *testing.T) { }{ { secretName: noAuthSecretName, - mockUsername: "username", - mockPassword: "password", - mockAuthMode: onvif.DigestAuth, + mockUsername: "", + mockPassword: "", + mockAuthMode: onvif.NoAuth, expected: Credentials{ AuthMode: AuthModeNone, }, @@ -109,28 +108,37 @@ func TestTryGetCredentials(t *testing.T) { } driver, mockService := createDriverWithMockService() - mockSecretProvider := &mocks.SecretProvider{} mockService.On("SecretProvider").Return(mockSecretProvider) + lookups := 3 for _, test := range tests { test := test t.Run(test.secretName, func(t *testing.T) { + // reset the secret provider + *mockSecretProvider = mocks.SecretProvider{} + if test.errorExpected { - mockSecretProvider.On("GetSecret", test.secretName, UsernameKey, PasswordKey, AuthModeKey).Return(nil, errors.NewCommonEdgeX(errors.KindServerError, "unit test error", nil)).Once() - } else { - mockSecretProvider.On("GetSecret", test.secretName, UsernameKey, PasswordKey, AuthModeKey).Return(map[string]string{"username": test.mockUsername, "password": test.mockPassword, "mode": test.mockAuthMode}, nil).Once() + mockSecretProvider.On("GetSecret", test.secretName, UsernameKey, PasswordKey, AuthModeKey). + Return(nil, errors.NewCommonEdgeX(errors.KindServerError, "unit test error", nil)). + Times(lookups) // expect to be called for every lookup + } else if test.secretName != noAuthSecretName { // expect not to be called for noAuth + mockSecretProvider.On("GetSecret", test.secretName, UsernameKey, PasswordKey, AuthModeKey). + Return(map[string]string{"username": test.mockUsername, "password": test.mockPassword, "mode": test.mockAuthMode}, nil). + Once() // expect to only be called once (cached lookups afterward) } - actual, err := driver.tryGetCredentials(test.secretName) - if test.errorExpected { - require.Error(t, err) - return + // perform the lookup multiple times to check caching + for i := 0; i < lookups; i++ { + actual, err := driver.getCredentials(test.secretName) + if test.errorExpected { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, test.expected, actual) } - require.NoError(t, err) - assert.Equal(t, test.expected.Username, actual.Username) - assert.Equal(t, test.expected.Password, actual.Password) - assert.Equal(t, test.expected.AuthMode, actual.AuthMode) + mockSecretProvider.AssertExpectations(t) }) } } @@ -187,7 +195,6 @@ func TestTryGetCredentialsForDevice(t *testing.T) { driver.macAddressMapper.credsMap = convertMACMappings(t, map[string]string{ "secret_name": "aa:bb:cc:dd:ee:ff", }) - driver.configMu = new(sync.RWMutex) driver.config = &ServiceConfig{ AppCustom: CustomConfig{ DefaultSecretName: "default_secret_name", @@ -195,23 +202,23 @@ func TestTryGetCredentialsForDevice(t *testing.T) { } mockSecretProvider := &mocks.SecretProvider{} - - for i := range tests { - if tests[i].errorExpected { - mockSecretProvider.On("GetSecret", tests[i].secretName, UsernameKey, PasswordKey, AuthModeKey).Return(nil, errors.NewCommonEdgeX(errors.KindServerError, "unit test error", nil)).Once() - } else { - mockSecretProvider.On("GetSecret", tests[i].secretName, UsernameKey, PasswordKey, AuthModeKey).Return(map[string]string{"username": tests[i].username, "password": tests[i].password, "mode": tests[i].authMode}, nil).Once() - } - } - mockService.On("SecretProvider").Return(mockSecretProvider) for _, test := range tests { test := test t.Run(test.secretName, func(t *testing.T) { + // reset the secret provider + *mockSecretProvider = mocks.SecretProvider{} - actual, err := driver.tryGetCredentialsForDevice(createTestDeviceWithProtocols(test.existingProtocols)) + if test.errorExpected { + mockSecretProvider.On("GetSecret", test.secretName, UsernameKey, PasswordKey, AuthModeKey). + Return(nil, errors.NewCommonEdgeX(errors.KindServerError, "unit test error", nil)) + } else { + mockSecretProvider.On("GetSecret", test.secretName, UsernameKey, PasswordKey, AuthModeKey). + Return(map[string]string{"username": test.username, "password": test.password, "mode": test.authMode}, nil) + } + actual, err := driver.tryGetCredentialsForDevice(createTestDeviceWithProtocols(test.existingProtocols)) if test.errorExpected { require.Error(t, err) return diff --git a/internal/driver/driver.go b/internal/driver/driver.go index ce63306f..ebfe8271 100644 --- a/internal/driver/driver.go +++ b/internal/driver/driver.go @@ -12,7 +12,7 @@ import ( "encoding/base64" "encoding/json" "fmt" - "net/http" + "github.com/edgexfoundry/go-mod-bootstrap/v3/bootstrap/secret" "net/url" "strings" "sync" @@ -27,7 +27,6 @@ import ( "github.com/edgexfoundry/go-mod-core-contracts/v3/errors" "github.com/edgexfoundry/go-mod-core-contracts/v3/models" - "github.com/IOTechSystems/onvif" onvifdevice "github.com/IOTechSystems/onvif/device" wsdiscovery "github.com/IOTechSystems/onvif/ws-discovery" ) @@ -45,17 +44,14 @@ const ( // Driver implements the sdkModel.ProtocolDriver interface for // the device service type Driver struct { - lc logger.LoggingClient - asynchCh chan<- *sdkModel.AsyncValues - deviceCh chan<- []sdkModel.DiscoveredDevice - + lc logger.LoggingClient sdkService interfaces.DeviceServiceSDK onvifClients map[string]*OnvifClient - clientsMu *sync.RWMutex + clientsMu sync.RWMutex config *ServiceConfig - configMu *sync.RWMutex + configMu sync.RWMutex macAddressMapper *MACAddressMapper @@ -63,11 +59,24 @@ type Driver struct { debounceTimer *time.Timer debounceMu sync.Mutex + // credsCache contains the cached credential information + credsCache map[string]Credentials + credsCacheMu sync.RWMutex + // taskCh is used to send signals to the taskLoop taskCh chan struct{} wg sync.WaitGroup } +func NewDriver() *Driver { + return &Driver{ + onvifClients: make(map[string]*OnvifClient), + config: &ServiceConfig{}, + credsCache: make(map[string]Credentials), + taskCh: make(chan struct{}), + } +} + type MultiErr []error //goland:noinspection GoReceiverNames @@ -83,16 +92,9 @@ func (me MultiErr) Error() string { // Initialize performs protocol-specific initialization for the device // service. func (d *Driver) Initialize(sdk interfaces.DeviceServiceSDK) error { - d.lc = sdk.LoggingClient() - d.asynchCh = sdk.AsyncValuesChannel() - d.deviceCh = sdk.DiscoveredDeviceChannel() - d.taskCh = make(chan struct{}) - d.clientsMu = new(sync.RWMutex) - d.configMu = new(sync.RWMutex) - d.onvifClients = make(map[string]*OnvifClient) d.sdkService = sdk - d.macAddressMapper = NewMACAddressMapper(d.sdkService) - d.config = &ServiceConfig{} + d.lc = sdk.LoggingClient() + d.macAddressMapper = NewMACAddressMapper(sdk) err := d.sdkService.LoadCustomConfig(d.config, "AppCustom") if err != nil { @@ -106,6 +108,11 @@ func (d *Driver) Initialize(sdk interfaces.DeviceServiceSDK) error { d.config.AppCustom.DiscoveryMode) } + err = d.sdkService.SecretProvider().RegisterSecretUpdatedCallback(secret.WildcardName, d.secretUpdated) + if err != nil { + d.lc.Errorf("failed to register secret update callback: %v", err) + } + d.macAddressMapper.UpdateMappings(d.config.AppCustom.CredentialsMap) err = d.sdkService.ListenForCustomConfigChanges(&d.config.AppCustom, "AppCustom", d.updateWritableConfig) @@ -115,19 +122,15 @@ func (d *Driver) Initialize(sdk interfaces.DeviceServiceSDK) error { for _, device := range d.sdkService.Devices() { d.lc.Infof("Initializing onvif client for '%s' camera", device.Name) - d.checkStatusOfDevice(device) - onvifClient, err := d.newOnvifClient(device) + _, err := d.createOnvifClient(device.Name, device.Protocols) if err != nil { d.lc.Errorf("failed to initialize onvif client for '%s' camera, skipping this device.", device.Name) continue } - d.clientsMu.Lock() - d.onvifClients[device.Name] = onvifClient - - d.clientsMu.Unlock() + d.checkStatusOfDevice(device) } - handler := NewRestNotificationHandler(d.sdkService, d.lc, d.asynchCh) + handler := NewRestNotificationHandler(d.sdkService) edgexErr := handler.AddRoute() if edgexErr != nil { return errors.NewCommonEdgeXWrapper(edgexErr) @@ -185,7 +188,8 @@ func (d *Driver) updateWritableConfig(rawWritableConfig interface{}) { func (d *Driver) debouncedDiscover() { d.lc.Debugf("trigger debounced discovery in %v", discoverDebounceDuration) - // everything in this function is mutex-locked and is safe to access asynchronously + // everything in this function is m + //utex-locked and is safe to access asynchronously d.debounceMu.Lock() defer d.debounceMu.Unlock() @@ -216,20 +220,16 @@ func (d *Driver) debouncedDiscover() { } } -func (d *Driver) getOnvifClient(deviceName string) (*OnvifClient, errors.EdgeX) { +func (d *Driver) getOnvifClient(device models.Device) (*OnvifClient, errors.EdgeX) { d.clientsMu.RLock() - defer d.clientsMu.RUnlock() - onvifClient, ok := d.onvifClients[deviceName] + onvifClient, ok := d.onvifClients[device.Name] + d.clientsMu.RUnlock() + var err error if !ok { - device, err := d.sdkService.GetDeviceByName(deviceName) - if err != nil { - return nil, errors.NewCommonEdgeXWrapper(err) - } - onvifClient, err = d.newOnvifClient(device) + onvifClient, err = d.createOnvifClient(device.Name, device.Protocols) if err != nil { return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("failed to initialize onvif client for '%s' camera", device.Name), err) } - d.onvifClients[deviceName] = onvifClient } return onvifClient, nil } @@ -246,7 +246,7 @@ func (d *Driver) HandleReadCommands(deviceName string, protocols map[string]mode var edgexErr errors.EdgeX var responses = make([]*sdkModel.CommandValue, len(reqs)) - onvifClient, edgexErr := d.getOnvifClient(deviceName) + onvifClient, edgexErr := d.getOnvifClient(models.Device{Name: deviceName, Protocols: protocols}) if edgexErr != nil { return responses, errors.NewCommonEdgeXWrapper(edgexErr) } @@ -299,7 +299,7 @@ func parametersFromURLRawQuery(req sdkModel.CommandRequest) ([]byte, errors.Edge func (d *Driver) HandleWriteCommands(deviceName string, protocols map[string]models.ProtocolProperties, reqs []sdkModel.CommandRequest, params []*sdkModel.CommandValue) error { var edgexErr errors.EdgeX - onvifClient, edgexErr := d.getOnvifClient(deviceName) + onvifClient, edgexErr := d.getOnvifClient(models.Device{Name: deviceName, Protocols: protocols}) if edgexErr != nil { return errors.NewCommonEdgeXWrapper(edgexErr) } @@ -341,7 +341,9 @@ func (d *Driver) HandleWriteCommands(deviceName string, protocols map[string]mod // for closing any in-use channels, including the channel used to send async // readings (if supported). func (d *Driver) Stop(force bool) error { - close(d.asynchCh) + if d.sdkService.AsyncValuesChannel() != nil { + close(d.sdkService.AsyncValuesChannel()) + } for _, client := range d.onvifClients { client.pullPointManager.UnsubscribeAll() client.baseNotificationManager.UnsubscribeAll() @@ -356,13 +358,9 @@ func (d *Driver) Stop(force bool) error { // AddDevice is a callback function that is invoked // when a new Device associated with this Device Service is added func (d *Driver) AddDevice(deviceName string, protocols map[string]models.ProtocolProperties, adminState models.AdminState) error { - device, err := d.sdkService.GetDeviceByName(deviceName) - if err != nil { - return errors.NewCommonEdgeXWrapper(err) - } - d.checkStatusOfDevice(device) // check the status of the newly added device + d.checkStatusOfDevice(models.Device{Name: deviceName, Protocols: protocols}) // check the status of the newly added device - err = d.createOnvifClient(deviceName) + _, err := d.createOnvifClient(deviceName, protocols) if err != nil { return errors.NewCommonEdgeXWrapper(err) } @@ -373,12 +371,8 @@ func (d *Driver) AddDevice(deviceName string, protocols map[string]models.Protoc // UpdateDevice is a callback function that is invoked // when a Device associated with this Device Service is updated func (d *Driver) UpdateDevice(deviceName string, protocols map[string]models.ProtocolProperties, adminState models.AdminState) error { - // Invoke the createOnvifClient func to create new onvif client and replace the old one - err := d.createOnvifClient(deviceName) - if err != nil { - return errors.NewCommonEdgeXWrapper(err) - } - return nil + // Invoke the updateOnvifClient func to update the old onvif client if needed + return d.updateOnvifClient(models.Device{Name: deviceName, Protocols: protocols}) } // RemoveDevice is a callback function that is invoked @@ -389,20 +383,16 @@ func (d *Driver) RemoveDevice(deviceName string, protocols map[string]models.Pro } // createOnvifClient creates the Onvif client used to communicate with the specified the device -func (d *Driver) createOnvifClient(deviceName string) error { - device, err := d.sdkService.GetDeviceByName(deviceName) +func (d *Driver) createOnvifClient(deviceName string, protocols map[string]models.ProtocolProperties) (*OnvifClient, errors.EdgeX) { + onvifClient, err := d.newOnvifClient(models.Device{Name: deviceName, Protocols: protocols}, false) if err != nil { - return errors.NewCommonEdgeXWrapper(err) - } - onvifClient, err := d.newOnvifClient(device) - if err != nil { - return errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("failed to initialize onvif client for '%s' camera", device.Name), err) + return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("failed to initialize onvif client for '%s' camera", deviceName), err) } d.clientsMu.Lock() defer d.clientsMu.Unlock() d.onvifClients[deviceName] = onvifClient - return nil + return onvifClient, nil } // Discover performs a discovery on the network and passes them to EdgeX to get provisioned @@ -437,7 +427,7 @@ func (d *Driver) Discover() error { // pass the discovered devices to the EdgeX SDK to be passed through to the provision watchers filtered := d.discoverFilter(discoveredDevices) - d.deviceCh <- filtered + d.sdkService.DiscoveredDeviceChannel() <- filtered return nil } @@ -508,118 +498,34 @@ func addressAndPort(xaddr string) (string, string) { } } -// todo: this should be integrated better with getDeviceInformation to avoid creating another temporary client -func (d *Driver) getNetworkInterfaces(device models.Device) (netInfo *onvifdevice.GetNetworkInterfacesResponse, edgexErr errors.EdgeX) { - devClient, edgexErr := d.newTemporaryOnvifClient(device) - if edgexErr != nil { - return nil, errors.NewCommonEdgeXWrapper(edgexErr) - } - devInfoResponse, edgexErr := devClient.callOnvifFunction(onvif.DeviceWebService, onvif.GetNetworkInterfaces, []byte{}) - if edgexErr != nil { - return nil, errors.NewCommonEdgeXWrapper(edgexErr) - } - devInfo, ok := devInfoResponse.(*onvifdevice.GetNetworkInterfacesResponse) - if !ok { - return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("invalid GetNetworkInterfacesResponse of type %T for the camera %s", devInfoResponse, device.Name), nil) - } - return devInfo, nil -} - -func (d *Driver) getDeviceInformation(device models.Device) (devInfo *onvifdevice.GetDeviceInformationResponse, edgexErr errors.EdgeX) { - devClient, edgexErr := d.newTemporaryOnvifClient(device) - if edgexErr != nil { - return nil, errors.NewCommonEdgeXWrapper(edgexErr) - } - devInfoResponse, edgexErr := devClient.callOnvifFunction(onvif.DeviceWebService, onvif.GetDeviceInformation, []byte{}) - if edgexErr != nil { - return nil, errors.NewCommonEdgeXWrapper(edgexErr) - } - devInfo, ok := devInfoResponse.(*onvifdevice.GetDeviceInformationResponse) - if !ok { - return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("invalid GetDeviceInformationResponse of type %T for the camera %s", devInfoResponse, device.Name), nil) - } - return devInfo, nil -} - -func (d *Driver) getEndpointReference(device models.Device) (devInfo *onvifdevice.GetEndpointReferenceResponse, edgexErr errors.EdgeX) { - devClient, edgexErr := d.newTemporaryOnvifClient(device) - if edgexErr != nil { - return nil, errors.NewCommonEdgeXWrapper(edgexErr) - } - endpointRefResponse, edgexErr := devClient.callOnvifFunction(onvif.DeviceWebService, onvif.GetEndpointReference, []byte{}) - if edgexErr != nil { - return nil, errors.NewCommonEdgeXWrapper(edgexErr) - } - devEndpointRef, ok := endpointRefResponse.(*onvifdevice.GetEndpointReferenceResponse) - if !ok { - return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("invalid GetEndpointReferenceResponse of type %T for the camera %s", endpointRefResponse, device.Name), nil) - } - return devEndpointRef, nil -} - -// newOnvifClient creates a temporary client for auto-discovery -func (d *Driver) newTemporaryOnvifClient(device models.Device) (*OnvifClient, errors.EdgeX) { - xAddr, edgexErr := GetCameraXAddr(device.Protocols) - if edgexErr != nil { - return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("failed to create cameraInfo for camera %s", device.Name), edgexErr) - } - - credential, edgexErr := d.tryGetCredentialsForDevice(device) - if edgexErr != nil { - // if credentials are not found, instead of returning an error, set the AuthMode to NoAuth - // and allow the user to call unauthenticated endpoints - d.lc.Warnf("failed to get credentials for camera %s, setting AuthMode to none for temporary client", device.Name) - credential = noAuthCredentials - } - - d.configMu.Lock() - requestTimeout := d.config.AppCustom.RequestTimeout - d.configMu.Unlock() - - onvifDevice, err := onvif.NewDevice(onvif.DeviceParams{ - Xaddr: xAddr, - Username: credential.Username, - Password: credential.Password, - AuthMode: credential.AuthMode, - HttpClient: &http.Client{ - Timeout: time.Duration(requestTimeout) * time.Second, - }, - }) - if err != nil { - return nil, errors.NewCommonEdgeX(errors.KindServiceUnavailable, "failed to initialize Onvif device client", err) - } - - client := &OnvifClient{ - lc: d.lc, - DeviceName: device.Name, - onvifDevice: onvifDevice, - } - return client, nil -} - // refreshDevice will attempt to retrieve the MAC address and the device info for the specified camera // and update the values in the protocol properties // Also the device name is updated if the name starts with the UnknownDevicePrefix and the status is UpWithAuth func (d *Driver) refreshDevice(device models.Device) error { + onvifClient, edgexErr := d.newOnvifClient(device, true) + if edgexErr != nil { + return edgexErr + } + // save the MAC Address in case it was changed by the calling code hwAddress := device.Protocols[OnvifProtocol][MACAddress] - devInfo, err := d.getDeviceInformation(device) + devInfo, err := onvifClient.getDeviceInformation(device) if err != nil { return err } - netInfo, netErr := d.getNetworkInterfaces(device) + netInfo, netErr := onvifClient.getNetworkInterfaces(device) if netErr != nil { d.lc.Warnf("Error trying to get network interfaces for device %s: %s", device.Name, netErr.Error()) } - endpointRef, endpointErr := d.getEndpointReference(device) + endpointRef, endpointErr := onvifClient.getEndpointReference(device) if endpointErr != nil { d.lc.Warnf("Error trying to get get endpoint reference for device %s: %s", device.Name, endpointErr.Error()) } - // update device to latest version in cache to prevent race conditions + // update device to latest version in cache to prevent race conditions and ensure we have all associated metadata device, edgeXErr := d.sdkService.GetDeviceByName(device.Name) if err != nil { return edgeXErr diff --git a/internal/driver/driver_test.go b/internal/driver/driver_test.go index b98a33d5..66d9caf9 100644 --- a/internal/driver/driver_test.go +++ b/internal/driver/driver_test.go @@ -13,7 +13,6 @@ import ( "net/http" "net/http/httptest" "strings" - "sync" "testing" "github.com/IOTechSystems/onvif" @@ -50,7 +49,9 @@ func boolPointer(val bool) *xsd.Boolean { func createDriverWithMockService() (*Driver, *sdkMocks.DeviceServiceSDK) { mockService := &sdkMocks.DeviceServiceSDK{} - driver := &Driver{sdkService: mockService, lc: logger.MockLogger{}} + driver := NewDriver() + driver.lc = logger.MockLogger{} + driver.sdkService = mockService return driver, mockService } @@ -111,7 +112,6 @@ func TestAddressAndPort(t *testing.T) { func TestDriver_HandleReadCommands(t *testing.T) { driver, mockService := createDriverWithMockService() - driver.clientsMu = new(sync.RWMutex) tests := []struct { name string @@ -376,7 +376,6 @@ func TestUpdateDevice(t *testing.T) { func TestDriver_RemoveDevice(t *testing.T) { driver, _ := createDriverWithMockService() - driver.clientsMu = new(sync.RWMutex) driver.onvifClients = map[string]*OnvifClient{ testDeviceName: {}, } diff --git a/internal/driver/macmapper.go b/internal/driver/macmapper.go index 09e09a5b..b8b8f072 100644 --- a/internal/driver/macmapper.go +++ b/internal/driver/macmapper.go @@ -47,9 +47,12 @@ func (m *MACAddressMapper) UpdateMappings(raw map[string]string) { } for _, mac := range strings.Split(macs, ",") { + if mac == "" { + continue // skip empty MAC addresses + } sanitized, err := SanitizeMACAddress(mac) if err != nil { - m.sdkService.LoggingClient().Warnf("Skipping invalid mac address %s: %s", mac, err.Error()) + m.sdkService.LoggingClient().Warnf("Skipping invalid mac address '%s': %s", mac, err.Error()) continue } // note: if the mac address already has a mapping, we do not overwrite it @@ -80,6 +83,8 @@ func (m *MACAddressMapper) TryGetSecretNameForMACAddress(mac string, defaultSecr secretName, found := m.credsMap[sanitized] if !found { m.sdkService.LoggingClient().Debugf("No credential mapping exists for mac address '%s', will use default secret name.", mac) + // store in lookup table + m.credsMap[sanitized] = defaultSecretName return defaultSecretName } diff --git a/internal/driver/macmapper_test.go b/internal/driver/macmapper_test.go index e97910ee..62b8f776 100644 --- a/internal/driver/macmapper_test.go +++ b/internal/driver/macmapper_test.go @@ -10,7 +10,6 @@ package driver import ( "reflect" "strings" - "sync" "testing" "github.com/edgexfoundry/go-mod-bootstrap/v3/bootstrap/interfaces/mocks" @@ -320,7 +319,6 @@ func TestTryGetSecretNameForMACAddress(t *testing.T) { driver.macAddressMapper.credsMap = convertMACMappings(t, map[string]string{ "valid_secret_name": mappedMac, }) - driver.configMu = new(sync.RWMutex) driver.config = &ServiceConfig{ AppCustom: CustomConfig{ DefaultSecretName: defaultSecretName, diff --git a/internal/driver/onvifclient.go b/internal/driver/onvifclient.go index 832dfa79..38486908 100644 --- a/internal/driver/onvifclient.go +++ b/internal/driver/onvifclient.go @@ -53,8 +53,9 @@ type OnvifClient struct { baseNotificationManager *BaseNotificationManager } -// newOnvifClient returns an OnvifClient for a single camera -func (d *Driver) newOnvifClient(device models.Device) (*OnvifClient, errors.EdgeX) { +// newOnvifClient returns an OnvifClient for a single camera. If temporary is true, a temporary client for +// auto-discovery is created without the extra managers and resources of a normal client. +func (d *Driver) newOnvifClient(device models.Device, temporary bool) (*OnvifClient, errors.EdgeX) { xAddr, edgexErr := GetCameraXAddr(device.Protocols) if edgexErr != nil { return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("failed to create cameraInfo for camera %s", device.Name), edgexErr) @@ -62,6 +63,8 @@ func (d *Driver) newOnvifClient(device models.Device) (*OnvifClient, errors.Edge credential, edgexErr := d.tryGetCredentialsForDevice(device) if edgexErr != nil { + // if credentials are not found, instead of returning an error, set the AuthMode to NoAuth + // and allow the user to call unauthenticated endpoints d.lc.Warnf("Unable to find credentials for Device %s, reverting to no auth", device.Name) credential = noAuthCredentials } @@ -83,18 +86,22 @@ func (d *Driver) newOnvifClient(device models.Device) (*OnvifClient, errors.Edge return nil, errors.NewCommonEdgeX(errors.KindServiceUnavailable, "failed to initialize Onvif device client", err) } - resource, err := d.getCameraEventResourceByDeviceName(device.Name) + client := &OnvifClient{ + driver: d, + lc: d.lc, + DeviceName: device.Name, + onvifDevice: onvifDevice, + } + + if temporary { + return client, nil + } + + client.CameraEventResource, err = d.getCameraEventResourceByDeviceName(device.Name) if err != nil { return nil, errors.NewCommonEdgeXWrapper(err) } - client := &OnvifClient{ - driver: d, - lc: d.lc, - DeviceName: device.Name, - onvifDevice: onvifDevice, - CameraEventResource: resource, - } // Create PullPointManager to control multiple pull points pullPointManager := newPullPointManager(d.lc) client.pullPointManager = pullPointManager @@ -105,6 +112,58 @@ func (d *Driver) newOnvifClient(device models.Device) (*OnvifClient, errors.Edge return client, nil } +// updateOnvifClient updates the internal onvifDevice of an onvif client +func (d *Driver) updateOnvifClient(device models.Device) errors.EdgeX { + xAddr, edgexErr := GetCameraXAddr(device.Protocols) + if edgexErr != nil { + return errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("failed to create cameraInfo for camera %s", device.Name), edgexErr) + } + + credential, edgexErr := d.tryGetCredentialsForDevice(device) + if edgexErr != nil { + d.lc.Warnf("Unable to find credentials for Device %s, reverting to no auth", device.Name) + credential = noAuthCredentials + } + + onvifClient, edgexErr := d.getOnvifClient(device) + if edgexErr == nil { + existingParams := onvifClient.onvifDevice.GetDeviceParams() + if xAddr == existingParams.Xaddr && credential.Username == existingParams.Username && + credential.Password == existingParams.Password && credential.AuthMode == existingParams.AuthMode { + // XAddr and credentials are the same, skip creating new connection + d.lc.Tracef("Skip creating new connection for un-modified device %s", device.Name) + return nil + } + } + + d.lc.Debugf("Updating connection for modified device %s", device.Name) + + d.configMu.Lock() + requestTimeout := d.config.AppCustom.RequestTimeout + d.configMu.Unlock() + + onvifDevice, err := onvif.NewDevice(onvif.DeviceParams{ + Xaddr: xAddr, + Username: credential.Username, + Password: credential.Password, + AuthMode: credential.AuthMode, + HttpClient: &http.Client{ + Timeout: time.Duration(requestTimeout) * time.Second, + }, + }) + if err != nil { + return errors.NewCommonEdgeX(errors.KindServiceUnavailable, "failed to update Onvif device client", err) + } + + // lock the clients to prevent access while the update occurs + d.clientsMu.Lock() + onvifClient.onvifDevice = onvifDevice + d.clientsMu.Unlock() + + d.checkStatusOfDevice(device) + return nil +} + func (d *Driver) getCameraEventResourceByDeviceName(deviceName string) (r models.DeviceResource, edgexErr errors.EdgeX) { device, err := d.sdkService.GetDeviceByName(deviceName) if err != nil { @@ -472,3 +531,39 @@ func (onvifClient *OnvifClient) checkRebootNeeded(responseContent interface{}) { return } } + +func (onvifClient *OnvifClient) getNetworkInterfaces(device models.Device) (netInfo *onvifdevice.GetNetworkInterfacesResponse, edgexErr errors.EdgeX) { + devInfoResponse, edgexErr := onvifClient.callOnvifFunction(onvif.DeviceWebService, onvif.GetNetworkInterfaces, []byte{}) + if edgexErr != nil { + return nil, errors.NewCommonEdgeXWrapper(edgexErr) + } + devInfo, ok := devInfoResponse.(*onvifdevice.GetNetworkInterfacesResponse) + if !ok { + return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("invalid GetNetworkInterfacesResponse of type %T for the camera %s", devInfoResponse, device.Name), nil) + } + return devInfo, nil +} + +func (onvifClient *OnvifClient) getDeviceInformation(device models.Device) (devInfo *onvifdevice.GetDeviceInformationResponse, edgexErr errors.EdgeX) { + devInfoResponse, edgexErr := onvifClient.callOnvifFunction(onvif.DeviceWebService, onvif.GetDeviceInformation, []byte{}) + if edgexErr != nil { + return nil, errors.NewCommonEdgeXWrapper(edgexErr) + } + devInfo, ok := devInfoResponse.(*onvifdevice.GetDeviceInformationResponse) + if !ok { + return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("invalid GetDeviceInformationResponse of type %T for the camera %s", devInfoResponse, device.Name), nil) + } + return devInfo, nil +} + +func (onvifClient *OnvifClient) getEndpointReference(device models.Device) (devInfo *onvifdevice.GetEndpointReferenceResponse, edgexErr errors.EdgeX) { + endpointRefResponse, edgexErr := onvifClient.callOnvifFunction(onvif.DeviceWebService, onvif.GetEndpointReference, []byte{}) + if edgexErr != nil { + return nil, errors.NewCommonEdgeXWrapper(edgexErr) + } + devEndpointRef, ok := endpointRefResponse.(*onvifdevice.GetEndpointReferenceResponse) + if !ok { + return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("invalid GetEndpointReferenceResponse of type %T for the camera %s", endpointRefResponse, device.Name), nil) + } + return devEndpointRef, nil +} diff --git a/internal/driver/onvifdiscovery.go b/internal/driver/onvifdiscovery.go index 9a8fc23e..a262e3ee 100644 --- a/internal/driver/onvifdiscovery.go +++ b/internal/driver/onvifdiscovery.go @@ -87,8 +87,8 @@ func (d *Driver) createDiscoveredDevice(onvifDevice onvif.Device) (sdkModel.Disc timestamp := time.Now().Format(time.UnixDate) device := contract.Device{ - // Using Xaddr as the temporary name - Name: xaddr, + // Using endpointRefAddr as the temporary name, since this is unique + Name: endpointRefAddr, Protocols: map[string]contract.ProtocolProperties{ OnvifProtocol: { Address: address, @@ -111,9 +111,14 @@ func (d *Driver) createDiscoveredDevice(onvifDevice onvif.Device) (sdkModel.Disc d.lc.Debugf("No MAC Address match was found for EndpointRefAddress %s", endpointRefAddr) } - devInfo, edgexErr := d.getDeviceInformation(device) + onvifClient, edgexErr := d.newOnvifClient(device, true) + if edgexErr != nil { + d.lc.Warnf("failed to create onvif client for the camera %s, %v", endpointRefAddr, edgexErr) + return sdkModel.DiscoveredDevice{}, fmt.Errorf("ailed to create onvif client for the camera %s", endpointRefAddr) + } var discovered sdkModel.DiscoveredDevice + devInfo, edgexErr := onvifClient.getDeviceInformation(device) if edgexErr != nil { d.lc.Warnf("failed to get the device information for the camera %s, %v", endpointRefAddr, edgexErr) device.Protocols[OnvifProtocol][DeviceStatus] = Reachable // update device status in this error case @@ -140,7 +145,7 @@ func (d *Driver) createDiscoveredDevice(onvifDevice onvif.Device) (sdkModel.Disc strings.ReplaceAll(strings.ReplaceAll(devInfo.Model, "/", "-"), " ", "-"), endpointRefAddr) - netInfo, err := d.getNetworkInterfaces(device) + netInfo, err := onvifClient.getNetworkInterfaces(device) if err != nil { d.lc.Warnf("failed to get the network information for device %s, %v", deviceName, edgexErr) } else { diff --git a/internal/driver/pullpointsubscriber.go b/internal/driver/pullpointsubscriber.go index 0c69fe56..50148b53 100644 --- a/internal/driver/pullpointsubscriber.go +++ b/internal/driver/pullpointsubscriber.go @@ -114,7 +114,7 @@ func (sub *Subscriber) pullMessage() errors.EdgeX { CommandValues: []*sdkModel.CommandValue{cv}, } - sub.onvifClient.driver.asynchCh <- asyncValues + sub.onvifClient.driver.sdkService.AsyncValuesChannel() <- asyncValues return nil } From 1ca05bc8a7440a63a166dfecc8b8679b98d0a0fe Mon Sep 17 00:00:00 2001 From: Anthony Casagrande Date: Wed, 3 May 2023 10:36:40 -0700 Subject: [PATCH 2/3] fix: address PR comments Signed-off-by: Anthony Casagrande --- Attribution.txt | 6 ++++++ go.mod | 2 +- go.sum | 4 ++-- internal/driver/credentials.go | 11 +++-------- internal/driver/onvifdiscovery.go | 2 +- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/Attribution.txt b/Attribution.txt index bffef725..f4d4b6b9 100644 --- a/Attribution.txt +++ b/Attribution.txt @@ -226,3 +226,9 @@ https://github.com/golang/tools/blob/master/LICENSE github.com/go-jose/go-jose/v3 (Apache-2.0) https://github.com/go-jose/go-jose https://github.com/go-jose/go-jose/blob/v3/LICENSE + +github.com/google/btree (Apache-2.0) https://github.com/google/btree +https://github.com/google/btree/blob/master/LICENSE + +github.com/kr/pretty (MIT) https://github.com/kr/pretty +https://github.com/kr/pretty/blob/main/License diff --git a/go.mod b/go.mod index 8a5e5a49..bebfcc06 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.20 require ( github.com/IOTechSystems/onvif v0.1.6 github.com/edgexfoundry/device-sdk-go/v3 v3.0.0-dev.77 - github.com/edgexfoundry/go-mod-bootstrap/v3 v3.0.0-dev.78 + github.com/edgexfoundry/go-mod-bootstrap/v3 v3.0.0-dev.79 github.com/edgexfoundry/go-mod-core-contracts/v3 v3.0.0-dev.39 github.com/google/uuid v1.3.0 github.com/gorilla/mux v1.8.0 diff --git a/go.sum b/go.sum index 1980c9ce..99129e01 100644 --- a/go.sum +++ b/go.sum @@ -36,8 +36,8 @@ github.com/eclipse/paho.mqtt.golang v1.4.2 h1:66wOzfUHSSI1zamx7jR6yMEI5EuHnT1G6r github.com/eclipse/paho.mqtt.golang v1.4.2/go.mod h1:JGt0RsEwEX+Xa/agj90YJ9d9DH2b7upDZMK9HRbFvCA= github.com/edgexfoundry/device-sdk-go/v3 v3.0.0-dev.77 h1:19MjelDoTw08STKmFM8UYN8FCAjy1bdx8FHEVpm539g= github.com/edgexfoundry/device-sdk-go/v3 v3.0.0-dev.77/go.mod h1:eqYSdDqo/7dNJ3eED50sMmiGCrk6nZNZMD+MJb3qfCk= -github.com/edgexfoundry/go-mod-bootstrap/v3 v3.0.0-dev.78 h1:1cj66uLeAswj01+krlmWP6+k+HSlZeaNRXET8bKf0/I= -github.com/edgexfoundry/go-mod-bootstrap/v3 v3.0.0-dev.78/go.mod h1:oXI84yFlBa+wNwX92tU7hYLaq/XwhUuBgiVGRLzZpMs= +github.com/edgexfoundry/go-mod-bootstrap/v3 v3.0.0-dev.79 h1:KRgTkaR36hfijNXfyPr0r/D0Ysc0GMinpC5CVu51ocs= +github.com/edgexfoundry/go-mod-bootstrap/v3 v3.0.0-dev.79/go.mod h1:oXI84yFlBa+wNwX92tU7hYLaq/XwhUuBgiVGRLzZpMs= github.com/edgexfoundry/go-mod-configuration/v3 v3.0.0-dev.10 h1:iDuAO3vpBQnlQuFhai/NATbJkiYXxo3bPCtSnFl07Yw= github.com/edgexfoundry/go-mod-configuration/v3 v3.0.0-dev.10/go.mod h1:8RlYm5CPzZgUsfXDWVP1TIeUMhsDNIdRdj1HXdomtOI= github.com/edgexfoundry/go-mod-core-contracts/v3 v3.0.0-dev.39 h1:tlw1Bd/hq4D6PQFo3BaN0JUkSqDuAPduaGBYCvoEYTg= diff --git a/internal/driver/credentials.go b/internal/driver/credentials.go index f11d3fce..c273c221 100644 --- a/internal/driver/credentials.go +++ b/internal/driver/credentials.go @@ -141,19 +141,14 @@ func (d *Driver) getCredentials(secretName string) (Credentials, errors.EdgeX) { return creds, nil } - d.credsCacheMu.Lock() creds, err := d.tryGetCredentialsInternal(secretName) - d.credsCacheMu.Unlock() if err != nil { return Credentials{}, err } + + d.credsCacheMu.Lock() // cache the credentials and return them d.credsCache[secretName] = creds + d.credsCacheMu.Unlock() return creds, nil } - -func (c Credentials) Equals(other Credentials) bool { - return c.Username == other.Username && - c.Password == other.Password && - c.AuthMode == other.AuthMode -} diff --git a/internal/driver/onvifdiscovery.go b/internal/driver/onvifdiscovery.go index a262e3ee..5fc414fe 100644 --- a/internal/driver/onvifdiscovery.go +++ b/internal/driver/onvifdiscovery.go @@ -114,7 +114,7 @@ func (d *Driver) createDiscoveredDevice(onvifDevice onvif.Device) (sdkModel.Disc onvifClient, edgexErr := d.newOnvifClient(device, true) if edgexErr != nil { d.lc.Warnf("failed to create onvif client for the camera %s, %v", endpointRefAddr, edgexErr) - return sdkModel.DiscoveredDevice{}, fmt.Errorf("ailed to create onvif client for the camera %s", endpointRefAddr) + return sdkModel.DiscoveredDevice{}, fmt.Errorf("failed to create onvif client for the camera %s", endpointRefAddr) } var discovered sdkModel.DiscoveredDevice From 8b4845b5a3078499d9da99f026e1eb8fcfaa7d59 Mon Sep 17 00:00:00 2001 From: Anthony Casagrande Date: Wed, 3 May 2023 14:45:10 -0700 Subject: [PATCH 3/3] refactor: tweak onvif client creation logic and remove secret cache secret cache was being used in a redundant manner, as secret store already caches some data. Signed-off-by: Anthony Casagrande --- internal/driver/checkstatuses.go | 40 +++----- internal/driver/credentials.go | 49 +++------- internal/driver/credentials_test.go | 147 ++++++++++++++++------------ internal/driver/driver.go | 65 +++--------- internal/driver/driver_test.go | 1 + internal/driver/onvifclient.go | 90 +++++++++++------ internal/driver/onvifdiscovery.go | 2 +- 7 files changed, 188 insertions(+), 206 deletions(-) diff --git a/internal/driver/checkstatuses.go b/internal/driver/checkstatuses.go index 58a8c5a8..d4795cf0 100644 --- a/internal/driver/checkstatuses.go +++ b/internal/driver/checkstatuses.go @@ -46,16 +46,15 @@ func (d *Driver) checkStatusOfDevice(device models.Device) { // if device is unknown, and missing a MAC Address, try and determine the MAC address via the endpoint reference if strings.HasPrefix(device.Name, UnknownDevicePrefix) && device.Protocols[OnvifProtocol][MACAddress] == "" { - endpointRefAddr := "" if v, ok := device.Protocols[OnvifProtocol][EndpointRefAddress]; ok { - endpointRefAddr = fmt.Sprintf("%v", v) - } - if endpointRefAddr != "" { - if mac := d.macAddressMapper.MatchEndpointRefAddressToMAC(endpointRefAddr); mac != "" { - // the mac address for the device was found, so set it here which will allow the - // code below to use the mac address for looking up the credentials. Because the mac mapper - // already contains them, the credentials will be found (whether they are valid or invalid). - device.Protocols[OnvifProtocol][MACAddress] = mac + endpointRefAddr := fmt.Sprintf("%v", v) + if endpointRefAddr != "" { + if mac := d.macAddressMapper.MatchEndpointRefAddressToMAC(endpointRefAddr); mac != "" { + // the mac address for the device was found, so set it here which will allow the + // code below to use the mac address for looking up the credentials. Because the mac mapper + // already contains them, the credentials will be found (whether they are valid or invalid). + device.Protocols[OnvifProtocol][MACAddress] = mac + } } } } @@ -82,7 +81,7 @@ func (d *Driver) checkStatusOfDevice(device models.Device) { // Higher degrees of connection are tested first, because if they // succeed, the lower levels of connection will too func (d *Driver) testConnectionMethods(device models.Device) (status string) { - devClient, err := d.getOnvifClient(device) + devClient, err := d.getOrCreateOnvifClient(device) if err != nil { d.lc.Warnf("Error getting onvif client for device %s", device.Name) // if we do not have a valid onvif client, lets just tcp probe it @@ -116,26 +115,13 @@ func (d *Driver) testConnectionMethods(device models.Device) (status string) { // tcpProbe attempts to make a connection to a specific ip and port list to determine // if there is a service listening at that ip+port. func (d *Driver) tcpProbe(device models.Device) bool { - proto, ok := device.Protocols[OnvifProtocol] - if !ok { - d.lc.Warnf("Device %s is missing required %s protocol info, cannot send probe.", device.Name, OnvifProtocol) + xAddr, edgexErr := GetCameraXAddr(device.Protocols) + if edgexErr != nil { + d.lc.Warnf("Device %s is missing required %s protocol info, cannot send probe: %v", device.Name, OnvifProtocol, edgexErr) return false } - addr := "" - if v, ok := proto[Address]; ok { - addr = fmt.Sprintf("%v", v) - } - port := "" - if v, ok := proto[Port]; ok { - port = fmt.Sprintf("%v", v) - } - if addr == "" || port == "" { - d.lc.Warnf("Device %s has no network address, cannot send probe.", device.Name) - return false - } - host := addr + ":" + port - conn, err := net.DialTimeout("tcp", host, time.Duration(d.config.AppCustom.ProbeTimeoutMillis)*time.Millisecond) + conn, err := net.DialTimeout("tcp", xAddr, time.Duration(d.config.AppCustom.ProbeTimeoutMillis)*time.Millisecond) if err != nil { d.lc.Debugf("Connection to %s failed when using simple tcp dial, Error: %s ", device.Name, err.Error()) return false diff --git a/internal/driver/credentials.go b/internal/driver/credentials.go index c273c221..9286aad3 100644 --- a/internal/driver/credentials.go +++ b/internal/driver/credentials.go @@ -84,11 +84,12 @@ func (d *Driver) tryGetCredentialsInternal(secretName string) (Credentials, erro return credentials, nil } -// tryGetCredentialsForDevice will attempt to use the device's MAC address to look up the credentials +// getCredentialsForDevice will attempt to use the device's MAC address to look up the credentials // from the Secret Store. If a mapping does not exist, or the device's MAC address is missing or invalid, -// the default secret name will be used to look up the credentials. An error is returned if the secret name -// does not exist in the Secret Store. -func (d *Driver) tryGetCredentialsForDevice(device models.Device) (Credentials, errors.EdgeX) { +// the default secret name will be used to look up the credentials. If the resolved secret name +// does not exist in the Secret Store, noAuthCredentials are returned, allowing the user +// to still call unauthenticated endpoints. +func (d *Driver) getCredentialsForDevice(device models.Device) Credentials { d.configMu.RLock() defaultSecretName := d.config.AppCustom.DefaultSecretName d.configMu.RUnlock() @@ -106,49 +107,25 @@ func (d *Driver) tryGetCredentialsForDevice(device models.Device) (Credentials, credentials, edgexErr := d.tryGetCredentialsInternal(secretName) if edgexErr != nil { - d.lc.Errorf("Failed to retrieve credentials for the secret name %s: %s", secretName, edgexErr.Error()) - return Credentials{}, errors.NewCommonEdgeX(errors.KindServerError, "failed to get credentials", edgexErr) + // if credentials are not found, instead of returning an error, set the AuthMode to NoAuth + // and allow the user to call unauthenticated endpoints + d.lc.Errorf("Failed to retrieve credentials for the secret name %s. Falling back to using NoAuth: %s", secretName, edgexErr.Error()) + return noAuthCredentials } - return credentials, nil + return credentials } func (d *Driver) secretUpdated(secretName string) { - d.lc.Infof("Secret updated callback called for secretName %s", secretName) - - d.credsCacheMu.Lock() - // remove the cache entry for this secret - delete(d.credsCache, secretName) - d.credsCacheMu.Unlock() - _, _ = d.getCredentials(secretName) // update cached data + d.lc.Infof("Secret updated callback called for secretName '%s'", secretName) for _, device := range d.sdkService.Devices() { - d.lc.Debugf("Updating onvif client for device %s", device.Name) + d.lc.Tracef("Updating onvif client for device %s", device.Name) err := d.updateOnvifClient(device) if err != nil { d.lc.Errorf("Unable to update onvif client for device: %s, %v", device.Name, err) } } - d.lc.Debug("Done updating onvif clients") -} - -func (d *Driver) getCredentials(secretName string) (Credentials, errors.EdgeX) { - d.credsCacheMu.RLock() - creds, found := d.credsCache[secretName] - d.credsCacheMu.RUnlock() - if found { - return creds, nil - } - - creds, err := d.tryGetCredentialsInternal(secretName) - if err != nil { - return Credentials{}, err - } - - d.credsCacheMu.Lock() - // cache the credentials and return them - d.credsCache[secretName] = creds - d.credsCacheMu.Unlock() - return creds, nil + d.lc.Trace("Done updating onvif clients") } diff --git a/internal/driver/credentials_test.go b/internal/driver/credentials_test.go index 63f362b5..6b5da258 100644 --- a/internal/driver/credentials_test.go +++ b/internal/driver/credentials_test.go @@ -8,6 +8,8 @@ package driver import ( + "fmt" + "github.com/stretchr/testify/mock" "testing" "github.com/IOTechSystems/onvif" @@ -18,6 +20,12 @@ import ( "github.com/stretchr/testify/require" ) +const ( + defaultSecretName = "default_secret_name" + secret1Name = "secret1" + testMAC1 = "" +) + // TestIsAuthModeValid verifies auth mode is set correctly. func TestIsAuthModeValid(t *testing.T) { @@ -125,12 +133,12 @@ func TestTryGetCredentials(t *testing.T) { } else if test.secretName != noAuthSecretName { // expect not to be called for noAuth mockSecretProvider.On("GetSecret", test.secretName, UsernameKey, PasswordKey, AuthModeKey). Return(map[string]string{"username": test.mockUsername, "password": test.mockPassword, "mode": test.mockAuthMode}, nil). - Once() // expect to only be called once (cached lookups afterward) + Times(lookups) // expect to be called for every lookup } - // perform the lookup multiple times to check caching + // perform the lookup multiple times for i := 0; i < lookups; i++ { - actual, err := driver.getCredentials(test.secretName) + actual, err := driver.tryGetCredentialsInternal(test.secretName) if test.errorExpected { require.Error(t, err) return @@ -143,49 +151,66 @@ func TestTryGetCredentials(t *testing.T) { } } -// TestTryGetCredentialsForDevice verifies correct credentials are returned for a device based on the MAC address of the device. -func TestTryGetCredentialsForDevice(t *testing.T) { +// TestGetCredentialsForDevice verifies correct credentials are returned for a device based on the MAC address of the device. +func TestGetCredentialsForDevice(t *testing.T) { + testMAC2 := "aa:bb:cc:11:11:11" + noAuthMAC := "cc:cc:dd:dd:11:22" + bogusMAC := "14:36:90:65:ff:ab" - tests := []struct { - existingProtocols map[string]models.ProtocolProperties - device models.Device - expected Credentials - secretName string + existingSecrets := map[string]Credentials{ + defaultSecretName: { + Username: "default", + Password: "password", + AuthMode: AuthModeBoth, + }, + secret1Name: { + Username: "user1", + Password: "pass1", + AuthMode: AuthModeUsernameToken, + }, + } - errorExpected bool - username string - password string - authMode string + tests := []struct { + name string + macAddress string + secretStore map[string]Credentials + expected Credentials }{ { - existingProtocols: map[string]models.ProtocolProperties{ - OnvifProtocol: { - MACAddress: "", - }, - }, - - secretName: "default_secret_name", - username: "username", - password: "password", - authMode: onvif.DigestAuth, - errorExpected: true, + name: "missing MAC, fallback to default credentials", + macAddress: "", + secretStore: existingSecrets, + expected: existingSecrets[defaultSecretName], }, { - existingProtocols: map[string]models.ProtocolProperties{ - OnvifProtocol: { - MACAddress: "aa:bb:cc:dd:ee:ff", - }, - }, - - secretName: "secret_name", - username: "username", - password: "password", - authMode: onvif.UsernameTokenAuth, - expected: Credentials{ - AuthMode: AuthModeUsernameToken, - Username: "username", - Password: "password", - }, + name: "no mapping for MAC, fallback to default credentials", + macAddress: bogusMAC, + secretStore: existingSecrets, + expected: existingSecrets[defaultSecretName], + }, + { + name: "success secret1", + macAddress: testMACAddress, + secretStore: existingSecrets, + expected: existingSecrets[secret1Name], + }, + { + name: "mapping points to missing secret, fallback to no auth", + macAddress: testMAC2, + secretStore: existingSecrets, + expected: noAuthCredentials, + }, + { + name: "no MAC specified, but missing default credentials secret, fallback to no auth", + macAddress: "", + secretStore: map[string]Credentials{}, + expected: noAuthCredentials, + }, + { + name: "success - explicitly map to no auth", + macAddress: noAuthMAC, + secretStore: map[string]Credentials{}, + expected: noAuthCredentials, }, } @@ -193,40 +218,40 @@ func TestTryGetCredentialsForDevice(t *testing.T) { driver.macAddressMapper = NewMACAddressMapper(mockService) driver.macAddressMapper.credsMap = convertMACMappings(t, map[string]string{ - "secret_name": "aa:bb:cc:dd:ee:ff", + secret1Name: testMACAddress, + "bogus": testMAC2, + noAuthSecretName: noAuthMAC, }) driver.config = &ServiceConfig{ AppCustom: CustomConfig{ - DefaultSecretName: "default_secret_name", + DefaultSecretName: defaultSecretName, }, } mockSecretProvider := &mocks.SecretProvider{} + getSecret := mockSecretProvider.On("GetSecret", mock.AnythingOfType("string"), UsernameKey, PasswordKey, AuthModeKey) mockService.On("SecretProvider").Return(mockSecretProvider) for _, test := range tests { test := test - t.Run(test.secretName, func(t *testing.T) { - // reset the secret provider - *mockSecretProvider = mocks.SecretProvider{} + t.Run(test.name, func(t *testing.T) { + // each run override the response of getSecret based on the provided secret store input + getSecret.Run(func(args mock.Arguments) { + if secret, ok := test.secretStore[args.String(0)]; ok { + getSecret.Return(map[string]string{"username": secret.Username, "password": secret.Password, "mode": secret.AuthMode}, nil) + } else { + getSecret.Return(nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("secret %s does not exist", args.String(0)), nil)) + } + }) - if test.errorExpected { - mockSecretProvider.On("GetSecret", test.secretName, UsernameKey, PasswordKey, AuthModeKey). - Return(nil, errors.NewCommonEdgeX(errors.KindServerError, "unit test error", nil)) - } else { - mockSecretProvider.On("GetSecret", test.secretName, UsernameKey, PasswordKey, AuthModeKey). - Return(map[string]string{"username": test.username, "password": test.password, "mode": test.authMode}, nil) - } + device := createTestDeviceWithProtocols(map[string]models.ProtocolProperties{ + OnvifProtocol: { + MACAddress: test.macAddress, + }, + }) - actual, err := driver.tryGetCredentialsForDevice(createTestDeviceWithProtocols(test.existingProtocols)) - if test.errorExpected { - require.Error(t, err) - return - } - require.NoError(t, err) - assert.Equal(t, test.expected.Username, actual.Username) - assert.Equal(t, test.expected.Password, actual.Password) - assert.Equal(t, test.expected.AuthMode, actual.AuthMode) + actual := driver.getCredentialsForDevice(device) + assert.Equal(t, test.expected, actual) }) } } diff --git a/internal/driver/driver.go b/internal/driver/driver.go index ebfe8271..9390cf52 100644 --- a/internal/driver/driver.go +++ b/internal/driver/driver.go @@ -59,10 +59,6 @@ type Driver struct { debounceTimer *time.Timer debounceMu sync.Mutex - // credsCache contains the cached credential information - credsCache map[string]Credentials - credsCacheMu sync.RWMutex - // taskCh is used to send signals to the taskLoop taskCh chan struct{} wg sync.WaitGroup @@ -72,7 +68,6 @@ func NewDriver() *Driver { return &Driver{ onvifClients: make(map[string]*OnvifClient), config: &ServiceConfig{}, - credsCache: make(map[string]Credentials), taskCh: make(chan struct{}), } } @@ -122,7 +117,7 @@ func (d *Driver) Initialize(sdk interfaces.DeviceServiceSDK) error { for _, device := range d.sdkService.Devices() { d.lc.Infof("Initializing onvif client for '%s' camera", device.Name) - _, err := d.createOnvifClient(device.Name, device.Protocols) + _, err := d.getOrCreateOnvifClient(device) if err != nil { d.lc.Errorf("failed to initialize onvif client for '%s' camera, skipping this device.", device.Name) continue @@ -188,8 +183,7 @@ func (d *Driver) updateWritableConfig(rawWritableConfig interface{}) { func (d *Driver) debouncedDiscover() { d.lc.Debugf("trigger debounced discovery in %v", discoverDebounceDuration) - // everything in this function is m - //utex-locked and is safe to access asynchronously + // everything in this function is mutex-locked and is safe to access asynchronously d.debounceMu.Lock() defer d.debounceMu.Unlock() @@ -220,34 +214,14 @@ func (d *Driver) debouncedDiscover() { } } -func (d *Driver) getOnvifClient(device models.Device) (*OnvifClient, errors.EdgeX) { - d.clientsMu.RLock() - onvifClient, ok := d.onvifClients[device.Name] - d.clientsMu.RUnlock() - var err error - if !ok { - onvifClient, err = d.createOnvifClient(device.Name, device.Protocols) - if err != nil { - return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("failed to initialize onvif client for '%s' camera", device.Name), err) - } - } - return onvifClient, nil -} - -func (d *Driver) removeOnvifClient(deviceName string) { - d.clientsMu.Lock() - defer d.clientsMu.Unlock() - // note: delete on non-existing keys is a no-op - delete(d.onvifClients, deviceName) -} - // HandleReadCommands triggers a protocol Read operation for the specified device. func (d *Driver) HandleReadCommands(deviceName string, protocols map[string]models.ProtocolProperties, reqs []sdkModel.CommandRequest) ([]*sdkModel.CommandValue, error) { var edgexErr errors.EdgeX var responses = make([]*sdkModel.CommandValue, len(reqs)) - onvifClient, edgexErr := d.getOnvifClient(models.Device{Name: deviceName, Protocols: protocols}) + onvifClient, edgexErr := d.getOrCreateOnvifClient(models.Device{Name: deviceName, Protocols: protocols}) if edgexErr != nil { + d.lc.Errorf("Failed to retrieve onvif client for camera '%s'", deviceName) return responses, errors.NewCommonEdgeXWrapper(edgexErr) } @@ -299,7 +273,7 @@ func parametersFromURLRawQuery(req sdkModel.CommandRequest) ([]byte, errors.Edge func (d *Driver) HandleWriteCommands(deviceName string, protocols map[string]models.ProtocolProperties, reqs []sdkModel.CommandRequest, params []*sdkModel.CommandValue) error { var edgexErr errors.EdgeX - onvifClient, edgexErr := d.getOnvifClient(models.Device{Name: deviceName, Protocols: protocols}) + onvifClient, edgexErr := d.getOrCreateOnvifClient(models.Device{Name: deviceName, Protocols: protocols}) if edgexErr != nil { return errors.NewCommonEdgeXWrapper(edgexErr) } @@ -358,13 +332,13 @@ func (d *Driver) Stop(force bool) error { // AddDevice is a callback function that is invoked // when a new Device associated with this Device Service is added func (d *Driver) AddDevice(deviceName string, protocols map[string]models.ProtocolProperties, adminState models.AdminState) error { - d.checkStatusOfDevice(models.Device{Name: deviceName, Protocols: protocols}) // check the status of the newly added device - - _, err := d.createOnvifClient(deviceName, protocols) + _, err := d.getOrCreateOnvifClient(models.Device{Name: deviceName, Protocols: protocols}) if err != nil { + d.lc.Errorf("Failed to initialize onvif client for camera '%s'", deviceName) return errors.NewCommonEdgeXWrapper(err) } - + // check the status of the newly added device + d.checkStatusOfDevice(models.Device{Name: deviceName, Protocols: protocols}) return nil } @@ -372,7 +346,11 @@ func (d *Driver) AddDevice(deviceName string, protocols map[string]models.Protoc // when a Device associated with this Device Service is updated func (d *Driver) UpdateDevice(deviceName string, protocols map[string]models.ProtocolProperties, adminState models.AdminState) error { // Invoke the updateOnvifClient func to update the old onvif client if needed - return d.updateOnvifClient(models.Device{Name: deviceName, Protocols: protocols}) + err := d.updateOnvifClient(models.Device{Name: deviceName, Protocols: protocols}) + if err != nil { + d.lc.Errorf("Unable to update onvif device client for device %s, %v", deviceName, err) + } + return err } // RemoveDevice is a callback function that is invoked @@ -382,19 +360,6 @@ func (d *Driver) RemoveDevice(deviceName string, protocols map[string]models.Pro return nil } -// createOnvifClient creates the Onvif client used to communicate with the specified the device -func (d *Driver) createOnvifClient(deviceName string, protocols map[string]models.ProtocolProperties) (*OnvifClient, errors.EdgeX) { - onvifClient, err := d.newOnvifClient(models.Device{Name: deviceName, Protocols: protocols}, false) - if err != nil { - return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("failed to initialize onvif client for '%s' camera", deviceName), err) - } - - d.clientsMu.Lock() - defer d.clientsMu.Unlock() - d.onvifClients[deviceName] = onvifClient - return onvifClient, nil -} - // Discover performs a discovery on the network and passes them to EdgeX to get provisioned func (d *Driver) Discover() error { d.lc.Info("Discover was called.") @@ -502,7 +467,7 @@ func addressAndPort(xaddr string) (string, string) { // and update the values in the protocol properties // Also the device name is updated if the name starts with the UnknownDevicePrefix and the status is UpWithAuth func (d *Driver) refreshDevice(device models.Device) error { - onvifClient, edgexErr := d.newOnvifClient(device, true) + onvifClient, edgexErr := d.getOrCreateOnvifClient(device) if edgexErr != nil { return edgexErr } diff --git a/internal/driver/driver_test.go b/internal/driver/driver_test.go index 66d9caf9..281928b3 100644 --- a/internal/driver/driver_test.go +++ b/internal/driver/driver_test.go @@ -52,6 +52,7 @@ func createDriverWithMockService() (*Driver, *sdkMocks.DeviceServiceSDK) { driver := NewDriver() driver.lc = logger.MockLogger{} driver.sdkService = mockService + mockService.On("LoggingClient").Return(driver.lc).Maybe() return driver, mockService } diff --git a/internal/driver/onvifclient.go b/internal/driver/onvifclient.go index 38486908..2ec235d2 100644 --- a/internal/driver/onvifclient.go +++ b/internal/driver/onvifclient.go @@ -53,31 +53,35 @@ type OnvifClient struct { baseNotificationManager *BaseNotificationManager } -// newOnvifClient returns an OnvifClient for a single camera. If temporary is true, a temporary client for -// auto-discovery is created without the extra managers and resources of a normal client. -func (d *Driver) newOnvifClient(device models.Device, temporary bool) (*OnvifClient, errors.EdgeX) { +// newOnvifClient returns a new OnvifClient for communicating with a single camera with all of the additional +// managers and resources needed for normal operation. +func (d *Driver) newOnvifClient(device models.Device) (*OnvifClient, errors.EdgeX) { + return d.newOnvifClientInternal(device, false) +} + +// newTemporaryOnvifClient returns a new OnvifClient for communicating with a single camera, however +// it is created without the extra managers and resources of a normal client for use in auto-discovery. +func (d *Driver) newTemporaryOnvifClient(device models.Device) (*OnvifClient, errors.EdgeX) { + return d.newOnvifClientInternal(device, true) +} + +// newOnvifClientInternal creates either a normal or a temporary OnvifClient +func (d *Driver) newOnvifClientInternal(device models.Device, temporary bool) (*OnvifClient, errors.EdgeX) { xAddr, edgexErr := GetCameraXAddr(device.Protocols) if edgexErr != nil { return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("failed to create cameraInfo for camera %s", device.Name), edgexErr) } - credential, edgexErr := d.tryGetCredentialsForDevice(device) - if edgexErr != nil { - // if credentials are not found, instead of returning an error, set the AuthMode to NoAuth - // and allow the user to call unauthenticated endpoints - d.lc.Warnf("Unable to find credentials for Device %s, reverting to no auth", device.Name) - credential = noAuthCredentials - } - d.configMu.Lock() requestTimeout := d.config.AppCustom.RequestTimeout d.configMu.Unlock() + credentials := d.getCredentialsForDevice(device) onvifDevice, err := onvif.NewDevice(onvif.DeviceParams{ Xaddr: xAddr, - Username: credential.Username, - Password: credential.Password, - AuthMode: credential.AuthMode, + Username: credentials.Username, + Password: credentials.Password, + AuthMode: credentials.AuthMode, HttpClient: &http.Client{ Timeout: time.Duration(requestTimeout) * time.Second, }, @@ -119,21 +123,19 @@ func (d *Driver) updateOnvifClient(device models.Device) errors.EdgeX { return errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("failed to create cameraInfo for camera %s", device.Name), edgexErr) } - credential, edgexErr := d.tryGetCredentialsForDevice(device) + onvifClient, edgexErr := d.getOrCreateOnvifClient(device) if edgexErr != nil { - d.lc.Warnf("Unable to find credentials for Device %s, reverting to no auth", device.Name) - credential = noAuthCredentials - } - - onvifClient, edgexErr := d.getOnvifClient(device) - if edgexErr == nil { - existingParams := onvifClient.onvifDevice.GetDeviceParams() - if xAddr == existingParams.Xaddr && credential.Username == existingParams.Username && - credential.Password == existingParams.Password && credential.AuthMode == existingParams.AuthMode { - // XAddr and credentials are the same, skip creating new connection - d.lc.Tracef("Skip creating new connection for un-modified device %s", device.Name) - return nil - } + return edgexErr + } + + credentials := d.getCredentialsForDevice(device) + existingParams := onvifClient.onvifDevice.GetDeviceParams() + // check the internal parameters used when creating the onvif device vs the current ones + if xAddr == existingParams.Xaddr && credentials.Username == existingParams.Username && + credentials.Password == existingParams.Password && credentials.AuthMode == existingParams.AuthMode { + // XAddr and credentials are the same, skip creating new connection + d.lc.Tracef("Skip creating new connection for un-modified device %s", device.Name) + return nil } d.lc.Debugf("Updating connection for modified device %s", device.Name) @@ -144,9 +146,9 @@ func (d *Driver) updateOnvifClient(device models.Device) errors.EdgeX { onvifDevice, err := onvif.NewDevice(onvif.DeviceParams{ Xaddr: xAddr, - Username: credential.Username, - Password: credential.Password, - AuthMode: credential.AuthMode, + Username: credentials.Username, + Password: credentials.Password, + AuthMode: credentials.AuthMode, HttpClient: &http.Client{ Timeout: time.Duration(requestTimeout) * time.Second, }, @@ -164,6 +166,32 @@ func (d *Driver) updateOnvifClient(device models.Device) errors.EdgeX { return nil } +func (d *Driver) getOrCreateOnvifClient(device models.Device) (*OnvifClient, errors.EdgeX) { + d.clientsMu.RLock() + onvifClient, ok := d.onvifClients[device.Name] + d.clientsMu.RUnlock() + if ok { + return onvifClient, nil + } + + onvifClient, err := d.newOnvifClient(device) + if err != nil { + return nil, errors.NewCommonEdgeX(errors.KindServerError, fmt.Sprintf("failed to initialize onvif client for '%s' camera", device.Name), err) + } + + d.clientsMu.Lock() + d.onvifClients[device.Name] = onvifClient + d.clientsMu.Unlock() + return onvifClient, nil +} + +func (d *Driver) removeOnvifClient(deviceName string) { + d.clientsMu.Lock() + defer d.clientsMu.Unlock() + // note: delete on non-existing keys is a no-op + delete(d.onvifClients, deviceName) +} + func (d *Driver) getCameraEventResourceByDeviceName(deviceName string) (r models.DeviceResource, edgexErr errors.EdgeX) { device, err := d.sdkService.GetDeviceByName(deviceName) if err != nil { diff --git a/internal/driver/onvifdiscovery.go b/internal/driver/onvifdiscovery.go index 5fc414fe..7a151894 100644 --- a/internal/driver/onvifdiscovery.go +++ b/internal/driver/onvifdiscovery.go @@ -111,7 +111,7 @@ func (d *Driver) createDiscoveredDevice(onvifDevice onvif.Device) (sdkModel.Disc d.lc.Debugf("No MAC Address match was found for EndpointRefAddress %s", endpointRefAddr) } - onvifClient, edgexErr := d.newOnvifClient(device, true) + onvifClient, edgexErr := d.newTemporaryOnvifClient(device) if edgexErr != nil { d.lc.Warnf("failed to create onvif client for the camera %s, %v", endpointRefAddr, edgexErr) return sdkModel.DiscoveredDevice{}, fmt.Errorf("failed to create onvif client for the camera %s", endpointRefAddr)