enterprise: add config map backed history store #1253
Conversation
| @@ -105,7 +107,7 @@ func (h *History) GetLatest(pubKey *ecdsa.PublicKey) (*LatestTransition, error) | |||
| // verify the transaction signature or return the transaction. | |||
| func (h *History) HasLatest() (bool, error) { | |||
| if _, err := h.store.Get("transitions/latest"); err != nil { | |||
| if errors.Is(err, os.ErrNotExist) { | |||
| if errors.Is(err, os.ErrNotExist) || k8serrors.IsNotFound(err) { | |||
There was a problem hiding this comment.
This is leaking implementation details from behind the interface (has been doing so before your change). We could retroactively decide that Get is supposed to return an os.ErrNotExist and do so in the configmapstore. Or add the Has method to the interface. Or add a bool output to Get. I think I'd go with the os error, though.
| return &ConfigMapStore{ | ||
| client: client, | ||
| namespace: self.Namespace, | ||
| uid: self.GetOwnerReferences()[0].UID, |
There was a problem hiding this comment.
We should check the ownerRef for at least the appropriate kind.
|
|
||
| // Get the value for key. | ||
| func (s *ConfigMapStore) Get(key string) ([]byte, error) { | ||
| ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) |
There was a problem hiding this comment.
Declare a constant for the timeout.
| return | ||
| case event, ok := <-watcher.ResultChan(): | ||
| if !ok { | ||
| return |
There was a problem hiding this comment.
We should probably add a logger to this store, so that we don't just swallow watch errors here and below.
| } | ||
|
|
||
| func objectName(key string) string { | ||
| return fmt.Sprintf("contrast-store-%s-%s", filepath.Dir(key), filepath.Base(key)) |
There was a problem hiding this comment.
This assumes that paths always have one level of nesting, and that there's no overlap between dir names (foo/bar-baz vs. foo-bar/baz,). These are probably fine assumptions, but we should at least document the constraints on the interface and enforce them here. "Keys must consist of two alphanumeric identifiers separated by forward slash".
| defer cancel() | ||
|
|
||
| cm, err := s.client.CoreV1().ConfigMaps(s.namespace).Get(ctx, objectName(key), metav1.GetOptions{}) | ||
| if err != nil && !(errors.IsNotFound(err) && len(oldVal) == 0) { |
There was a problem hiding this comment.
We should compare oldVal == nil to distinguish empty value from absent value.
This implements the Store interface using Kubernetes Config Maps in preparation for the distributed Coordinator. For now, this can be tested by manually setting the
enterprisebuild tag during the build process.